[CHECKER] 9 potential out-of-bounds array access errors

Hi all,

We are a group of Stanford researchers, and we've recently developed a
tool that detects potential out-of-bounds array accesses and buffer
overruns. Here are 9 potential bugs we've found on postgresql 7.3.1.
We've been checking linux for a few years, and we're interested in
expanding to other system software as well. Let us know if you guys are
interested in bug reports like this. Confirmation and comments will be
appreciated.

Regards,
Yichen
Meta Compilation Group
http://metacomp.stanford.edu (little out of date tho)

############################################################
# New errors.
#
---------------------------------------------------------
[BUG] MAX_TIME_PRECISION defined to be 13 when HAVE_INT64_TIMESTAMP is
not defined
X [FALSE]
X [UNKNOWN]
X [BROKE]
X [SKIP]
/u2/yxie/postgresql-7.3.1/src/backend/utils/adt/date.c:682:AdjustTimeFor
Typmod: ERROR:BUFFER:682:682:Array bounds error (off >= len) [RANGE]
(TimeScales[typmod], len = 7, off = sym_905407, max(off-len) = 6)

}
#else
/* we have different truncation behavior depending on
sign */
if (*time >= 0)
{

Error --->
*time = (rint(((double) *time) *
TimeScales[typmod])
/ TimeScales[typmod]);
}
else
---------------------------------------------------------
[BUG] "i" can go up to 13
X [FALSE]
X [UNKNOWN]
X [BROKE]
X [SKIP]
/u2/yxie/postgresql-7.3.1/src/backend/utils/mb/conversion_procs/euc_tw_a
nd_big5/big5.c:364:CNStoBIG5: ERROR:BUFFER:364:364:Array bounds error
(off >= len) (b2c3[i], len = 7, off = 7, min(off-len) = 0)

big5 = BinarySearchRange(cnsPlane2ToBig5Level2,
47, cns);
break;
case LC_CNS11643_3:
for (i = 0; i < sizeof(b2c3) / sizeof(unsigned
short); i++)
{

Error --->
if (b2c3[i][1] == cns)
return (b2c3[i][0]);
}
break;
---------------------------------------------------------
[BUG] "i" can go up to 13
X [FALSE]
X [UNKNOWN]
X [BROKE]
X [SKIP]
/u2/yxie/postgresql-7.3.1/src/backend/utils/mb/conversion_procs/euc_tw_a
nd_big5/big5.c:371:CNStoBIG5: ERROR:BUFFER:371:371:Array bounds error
(off >= len) (b1c4[i], len = 4, off = 4, min(off-len) = 0)

}
break;
case LC_CNS11643_4:
for (i = 0; i < sizeof(b1c4) / sizeof(unsigned
short); i++)
{

Error --->
if (b1c4[i][1] == cns)
return (b1c4[i][0]);
}
default:
---------------------------------------------------------
[BUG] is plpgsql_nDatums 0 here? also, sizeof (plpgsql_nDatums) =
2*sizeof(PLpgSQL_datum *)
X [FALSE]
X [UNKNOWN]
X [BROKE]
X [SKIP]
/u2/yxie/postgresql-7.3.1/src/pl/plpgsql/src/pl_comp.c:527:plpgsql_compi
le: ERROR:BUFFER:527:527:Dereferencing uninitialized pointer
(*(*function).datums + (PLpgSQL_datum**)(Oid)i * 4) evaluated in the
following state

for (i = 0; i < function->fn_nargs; i++)
function->fn_argvarnos[i] = arg_varnos[i];
function->ndatums = plpgsql_nDatums;
function->datums = malloc(sizeof(PLpgSQL_datum *) *
plpgsql_nDatums);
for (i = 0; i < plpgsql_nDatums; i++)

Error --->
function->datums[i] = plpgsql_Datums[i];
function->action = plpgsql_yylval.program;

ReleaseSysCache(procTup);
---------------------------------------------------------
[BUG] does fe_setauthsvc abort the function? if not there's a
possibility of an overrun
X [FALSE]
X [UNKNOWN]
X [BROKE]
X [SKIP]
/u2/yxie/postgresql-7.3.1/src/interfaces/libpq/fe-auth.c:688:fe_getauths
vc: ERROR:BUFFER:688:688:Array bounds error (off >= len)
(authsvcs[pg_authsvc], len = 2, off = sym_3532626, min(off-len) = 0)

MsgType
fe_getauthsvc(char *PQerrormsg)
{
if (pg_authsvc < 0 || pg_authsvc >= n_authsvcs)
fe_setauthsvc(DEFAULT_CLIENT_AUTHSVC, PQerrormsg);

Error --->
return authsvcs[pg_authsvc].msgtype;
}

/*
---------------------------------------------------------
[BUG] "i" can go up to 13
X [FALSE]
X [UNKNOWN]
X [BROKE]
X [SKIP]
/u2/yxie/postgresql-7.3.1/src/backend/utils/mb/conversion_procs/euc_tw_a
nd_big5/big5.c:325:BIG5toCNS: ERROR:BUFFER:325:325:Array bounds error
(off >= len) (b2c3[i], len = 7, off = 7, min(off-len) = 0)

else
{
/* level 2 */
for (i = 0; i < sizeof(b2c3) / sizeof(unsigned short);
i++)
{

Error --->
if (b2c3[i][0] == big5)
{
*lc = LC_CNS11643_3;
return (b2c3[i][1] | 0x8080U);
---------------------------------------------------------
[BUG] MAX_TIME_PRECISION is 13
X [FALSE]
X [UNKNOWN]
X [BROKE]
X [SKIP]
/u2/yxie/postgresql-7.3.1/src/backend/utils/adt/date.c:691:AdjustTimeFor
Typmod: ERROR:BUFFER:691:691:Array bounds error (off >= len) [RANGE]
(TimeOffsets[typmod], len = 7, off = sym_905407, max(off-len) = 6)

{
/*
* Scale and truncate first, then add to help
the rounding
* behavior
*/

Error --->
*time = (rint((((double) *time) *
TimeScales[typmod]) + TimeOffsets[typmod])
/ TimeScales[typmod]);
}
#endif
---------------------------------------------------------
[BUG]
X [FALSE]
X [UNKNOWN]
X [BROKE]
X [SKIP]
/u2/yxie/postgresql-7.3.1/src/backend/utils/mb/conversion_procs/euc_tw_a
nd_big5/big5.c:304:BIG5toCNS: ERROR:BUFFER:304:304:Array bounds error
(off >= len) (b1c4[i], len = 4, off = 4, min(off-len) = 0)

{
/* level 1 */

for (i = 0; i < sizeof(b1c4) / sizeof(unsigned short);
i++)
{

Error --->
if (b1c4[i][0] == big5)
{
*lc = LC_CNS11643_4;
return (b1c4[i][1] | 0x8080U);
---------------------------------------------------------
[BUG] ndim can be 0...
X [FALSE]
X [UNKNOWN]
X [BROKE]
X [SKIP]
/u2/yxie/postgresql-7.3.1/src/backend/utils/adt/arrayfuncs.c:352:ArrayCo
unt: ERROR:BUFFER:352:352:Array bounds error (off < 0) (temp[ndim - 1],
max(off) = -1)

break;
}
if (!itemdone)
ptr++;
}

Error --->
temp[ndim - 1]++;
ptr++;
}
for (i = 0; i < ndim; ++i)

############################################################
# Existing, unfixed errors
#

############################################################
# Existing, skipped errors
#

############################################################
# Existing unknown
#

############################################################
# Existing false positives
#

############################################################

############################################################
# New Fixed errors
#

############################################################
# Old fixed
#

############################################################

# Summary for BUFFER
# New errors = 9
# Existing unfixed errors = 0
# Existing unfixed skip = 0
# Existing unknown = 0
# Existing false pos = 0
# Mismatch errors = 0
# Fixed errors = 0
# Fixed false/broke = 0
# Fixed unknown = 0
# Old fixed = 0

"Yichen Xie" <yxie(at)cs(dot)stanford(dot)edu> writes:
> We are a group of Stanford researchers, and we've recently developed a
> tool that detects potential out-of-bounds array accesses and buffer
> overruns. Here are 9 potential bugs we've found on postgresql 7.3.1.
> We've been checking linux for a few years, and we're interested in
> expanding to other system software as well. Let us know if you guys are
> interested in bug reports like this.

This looks like great stuff --- I haven't read through all of them, but
at least the first couple look like genuine bugs. I'm a little
suspicious of the tool's coverage though. For example, in
src/backend/utils/mb/conversion_procs/euc_tw_and_big5/big5.c,
why'd it flag only one of the two loops that use the same incorrect
limit for scanning b1c4[][] ?

regards, tom lane

Both are flagged though--the other one's 85 lines down in the bug report..
;) I probably should've sorted the list by location to minimize confusion.
Thanks for the feedback!

-Yichen

On Tue, 28 Jan 2003, Tom Lane wrote:

> This looks like great stuff --- I haven't read through all of them, but
> at least the first couple look like genuine bugs. I'm a little
> suspicious of the tool's coverage though. For example, in
> src/backend/utils/mb/conversion_procs/euc_tw_and_big5/big5.c,
> why'd it flag only one of the two loops that use the same incorrect
> limit for scanning b1c4[][] ?
>
> regards, tom lane
>

Yichen Xie <yxie(at)cs(dot)stanford(dot)edu> writes:
> Both are flagged though--the other one's 85 lines down in the bug report..
> ;) I probably should've sorted the list by location to minimize confusion.

That's okay, I probably should've read the whole mail before commenting ;-)

I'm confused by the entry flagging pl_comp.c:527:

[BUG] is plpgsql_nDatums 0 here? also, sizeof (plpgsql_nDatums) =
2*sizeof(PLpgSQL_datum *)

Is the thing concerned because malloc(0) may yield NULL on some
platforms? If so, should I object that it ought to be smart enough to
know the loop won't execute in that case? Or am I missing something?
Also, I don't understand your comment about the sizeof() relationship.

regards, tom lane

I think it's 'coz the only assignment to "plpgsql_nDatums" the checker
could find is on line 176, without realizing plpgsql_nDatums is actually a
global variable and could be changed anywhere... We'll rule out cases like
this in the future. Thanks for letting us know. --yichen

On Tue, 28 Jan 2003, Tom Lane wrote:

> I'm confused by the entry flagging pl_comp.c:527:
>
> [BUG] is plpgsql_nDatums 0 here? also, sizeof (plpgsql_nDatums) =
> 2*sizeof(PLpgSQL_datum *)
>
> Is the thing concerned because malloc(0) may yield NULL on some
> platforms? If so, should I object that it ought to be smart enough to
> know the loop won't execute in that case? Or am I missing something?
> Also, I don't understand your comment about the sizeof() relationship.
>
> regards, tom lane
>

> We are a group of Stanford researchers, and we've recently developed a
> tool that detects potential out-of-bounds array accesses and buffer
> overruns. Here are 9 potential bugs we've found on postgresql 7.3.1.
> We've been checking linux for a few years, and we're interested in
> expanding to other system software as well. Let us know if you guys are
> interested in bug reports like this. Confirmation and comments will be
> appreciated.

Thanks. I have confirmed that at least following reports are
correct. Will fix them.

> [BUG] "i" can go up to 13
> X [FALSE]
> X [UNKNOWN]
> X [BROKE]
> X [SKIP]
> /u2/yxie/postgresql-7.3.1/src/backend/utils/mb/conversion_procs/euc_tw_a
> nd_big5/big5.c:364:CNStoBIG5: ERROR:BUFFER:364:364:Array bounds error
v
> [BUG] "i" can go up to 13
> X [FALSE]
> X [UNKNOWN]
> X [BROKE]
> X [SKIP]
> /u2/yxie/postgresql-7.3.1/src/backend/utils/mb/conversion_procs/euc_tw_a
> nd_big5/big5.c:371:CNStoBIG5: ERROR:BUFFER:371:371:Array bounds error
--
Tatsuo Ishii

"Yichen Xie" <yxie(at)cs(dot)stanford(dot)edu> writes:
> We are a group of Stanford researchers, and we've recently developed a
> tool that detects potential out-of-bounds array accesses and buffer
> overruns. Here are 9 potential bugs we've found on postgresql 7.3.1.

Here's a status report:

> [BUG] MAX_TIME_PRECISION defined to be 13 when HAVE_INT64_TIMESTAMP is
> not defined
> /u2/yxie/postgresql-7.3.1/src/backend/utils/adt/date.c:682:AdjustTimeFor
> Typmod: ERROR:BUFFER:682:682:Array bounds error (off >= len) [RANGE]

Real bug introduced in multiple-time-storage-format changes in 7.3.
Fixed in current and 7.3 branch.

> [BUG] "i" can go up to 13
> /u2/yxie/postgresql-7.3.1/src/backend/utils/mb/conversion_procs/euc_tw_a
> nd_big5/big5.c:364:CNStoBIG5: ERROR:BUFFER:364:364:Array bounds error

Real bug, code is new in 7.3. Fixed in current and 7.3 branch.

> [BUG] "i" can go up to 13
> /u2/yxie/postgresql-7.3.1/src/backend/utils/mb/conversion_procs/euc_tw_a
> nd_big5/big5.c:371:CNStoBIG5: ERROR:BUFFER:371:371:Array bounds error

As above.

> [BUG] is plpgsql_nDatums 0 here? also, sizeof (plpgsql_nDatums) =
> 2*sizeof(PLpgSQL_datum *)
> /u2/yxie/postgresql-7.3.1/src/pl/plpgsql/src/pl_comp.c:527:plpgsql_compi
> le: ERROR:BUFFER:527:527:Dereferencing uninitialized pointer

Doesn't seem to be a bug, unless I'm missing something. Checker
apparently fooled by globalness of variable?

> [BUG] does fe_setauthsvc abort the function? if not there's a
> possibility of an overrun
> /u2/yxie/postgresql-7.3.1/src/interfaces/libpq/fe-auth.c:688:fe_getauths
> vc: ERROR:BUFFER:688:688:Array bounds error (off >= len)

Potential bug; could only trigger if compile-time-constant
DEFAULT_CLIENT_AUTHSVC has incorrect value. I wouldn't expect the
checker to realize that, though (it'd take cross-procedural analysis).
Fixed in CVS head in case of future mistakes, but not back-patched.

> [BUG] "i" can go up to 13
> /u2/yxie/postgresql-7.3.1/src/backend/utils/mb/conversion_procs/euc_tw_a
> nd_big5/big5.c:325:BIG5toCNS: ERROR:BUFFER:325:325:Array bounds error

See above.

> [BUG] MAX_TIME_PRECISION is 13
> /u2/yxie/postgresql-7.3.1/src/backend/utils/adt/date.c:691:AdjustTimeFor
> Typmod: ERROR:BUFFER:691:691:Array bounds error (off >= len) [RANGE]

See above.

> [BUG]
> /u2/yxie/postgresql-7.3.1/src/backend/utils/mb/conversion_procs/euc_tw_a
> nd_big5/big5.c:304:BIG5toCNS: ERROR:BUFFER:304:304:Array bounds error

See above.

> [BUG] ndim can be 0...
> /u2/yxie/postgresql-7.3.1/src/backend/utils/adt/arrayfuncs.c:352:ArrayCo
> unt: ERROR:BUFFER:352:352:Array bounds error (off < 0) (temp[ndim - 1],

This cannot happen in current sources because ArrayCount is only invoked
on strings beginning with '{'. Still, it seems like an accident waiting
to happen. I've modified CVS tip so that ndim is initialized to 1, not
0, to forestall any future problem.

Thanks for the report!

regards, tom lane

Tatsuo Ishii <t-ishii(at)sra(dot)co(dot)jp> writes:
> Thanks. I have confirmed that at least following reports are
> correct. Will fix them.

I already committed fixes in HEAD and 7.3 branch.

I wanted to ask you if this might explain some of the odd reports we've
gotten about conversion problems? I can't recall right now if any of
them had to do with big5 data ...

regards, tom lane

> Tatsuo Ishii <t-ishii(at)sra(dot)co(dot)jp> writes:
> > Thanks. I have confirmed that at least following reports are
> > correct. Will fix them.
>
> I already committed fixes in HEAD and 7.3 branch.

Thanks.

> I wanted to ask you if this might explain some of the odd reports we've
> gotten about conversion problems? I can't recall right now if any of
> them had to do with big5 data ...

I don't think so. It seems most of them were caused by bad input text
strings(incorrect EUC_TW or so).
--
Tatsuo Ishii