On Mon, 18 Feb 2008, Sebastian Gottschalk wrote:
> Dear Sir or Madam,
>
> I found some bugs in Pcre 7.6.
>
> 1. pcre_compile.c line 1970 accesses the variable 'othercase' which might be
> unintialized.
I'm afraid I disagree with you. The code is
1965 for (c = *cptr; c <= d; c++)
1966 { if ((othercase = _pcre_ucp_othercase(c)) != NOTACHAR) break; }
1967
1968 if (c > d) return FALSE;
1969
1970 *ocptr = othercase;
When line 1970 is reached, we know that c must be <= d (otherwise line 1968
would have terminated the function). If c <= d the loop starting in 1965
must have executed at least once, causing 'othercase' to be set.
> 2. pcreposix.c 3=lines 312,313 access 'ovector', which might still be NULL
> (even after the call to pcre_exec()). Line 316 accessed 'i', which might be
> uninitialized.
Again, I disagree. Early in pcreposix.c, we have:
if (nosub) nmatch = 0;
else if (nmatch > 0)
{
if (nmatch <= POSIX_MALLOC_THRESHOLD)
{
ovector = &(small_ovector[0]);
}
else
{
if (nmatch > INT_MAX/(sizeof(int) * 3)) return REG_ESPACE;
ovector = (int *)malloc(sizeof(int) * nmatch * 3);
if (ovector == NULL) return REG_ESPACE;
allocated_ovector = TRUE;
}
}
That code ensures that ovector is not NULL, unless nosub is TRUE. The
lines you complain about are in this code:
if (!nosub)
{
for (i = 0; i < (size_t)rc; i++)
{
pmatch[i].rm_so = ovector[i*2];
pmatch[i].rm_eo = ovector[i*2+1];
}
if (allocated_ovector) free(ovector);
for (; i < nmatch; i++) pmatch[i].rm_so = pmatch[i].rm_eo = -1;
}
Notice that the references to ovector are obeyed only if nosub is FALSE.
I don't agree about your comment for the variable 'i' either. That
variable is set to zero at the start of the for loop in line 309. (This
is C code - are you thinking of a language where a variable gets unset
at the end of a loop?)
> 3. pcrepp.c lines 657,665 call isspace() and isdigit() on a const char, but
> they require an unsigned char. Since their signature allows for 'int',
> there's no implicit conversion taking place.
I will pass that comment on to the maintainer of the C++ wrapper. I
myself am not a C++ programmer.
Thank you for taking the time to comment.
Philip
--
Philip Hazel
