Thread: [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@cs.stanford.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
Yichen Xie <yxie@cs.stanford.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
> 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@cs.stanford.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
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 >
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 >
Tatsuo Ishii <t-ishii@sra.co.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@sra.co.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