On 05/14/2015 01:47 PM, Held, Douglas wrote:
> <html><bodyHi SQLite,
>
> A team of mine wants to use SQLCipher so I scanned it with Fortify SCA.
> SQLCipher includes sqlite3.c version 3.8.8.3. The software has reported a
> Buffer Overflow (off-by-one) in the following C code:
>
> In sqlite3.c, it says the overflow can happen here on line 96492 when 'i'
> gets down to 1:
>
> 96487: SQLITE_PRIVATE void sqlite3SrcListShiftJoinType(SrcList *p){
> 96488: if( p ){
> 96489: int i;
> 96490: assert( p->a || p->nSrc==0 );
> 96491: for(i=p->nSrc-1; i>0; i--){
> 96492: p->a[i].jointype = p->a[i-1].jointype;
> 96493: }
> 96494: p->a[0].jointype = 0;
> 96495: }
> 96496: }
>
> The declaration of this buffer 'a' is on line 11973:
>
> 11946: struct SrcList {
> 11947: int nSrc; /* Number of tables or subqueries in the FROM
> clause */
> 11948: u32 nAlloc; /* Number of entries allocated in a[] below */
> 11949: struct SrcList_item {
> 11950: Schema *pSchema; /* Schema to which this item is fixed */
> 11951: char *zDatabase; /* Name of database holding this table */
> 11952: char *zName; /* Name of the table */
> 11953: char *zAlias; /* The "B" part of a "A AS B" phrase. zName is
> the "A" */
> 11954: Table *pTab; /* An SQL table corresponding to zName */
> 11955: Select *pSelect; /* A SELECT statement used in place of a table
> name */
> 11956: int addrFillSub; /* Address of subroutine to manifest a subquery
> */
> 11957: int regReturn; /* Register holding return address of
> addrFillSub */
> 11958: int regResult; /* Registers holding results of a co-routine */
> 11959: u8 jointype; /* Type of join between this able and the
> previous */
> 11960: unsigned notIndexed :1; /* True if there is a NOT INDEXED
> clause */
> 11961: unsigned isCorrelated :1; /* True if sub-query is correlated */
> 11962: unsigned viaCoroutine :1; /* Implemented as a co-routine */
> 11963: unsigned isRecursive :1; /* True for recursive reference in WITH
> */
> 11964: #ifndef SQLITE_OMIT_EXPLAIN
> 11965: u8 iSelectId; /* If pSelect!=0, the id of the sub-select in
> EQP */
> 11966: #endif
> 11967: int iCursor; /* The VDBE cursor number used to access this
> table */
> 11968: Expr *pOn; /* The ON clause of a join */
> 11969: IdList *pUsing; /* The USING clause of a join */
> 11970: Bitmask colUsed; /* Bit N (1<<N) set if column N of pTab is used
> */
> 11971: char *zIndex; /* Identifier from "INDEXED BY <zIndex>" clause
> */
> 11972: Index *pIndex; /* Index structure corresponding to zIndex, if
> any */
> 11973: } a[1]; /* One entry for each identifier on the list */
> 11974: };
>
> The analyzer says that the real length of this thing is 112 bytes long (can
> someone verify that?) and that up above on line 96492, the write length into
> the buffer is 224 bytes, at least when 'i' gets down to 1.
Perhaps "is 1 or greater" instead of "down to 1". I don't think it's an
actual problem. The static analyzer is assuming that all pointers of
type (SrcList*) point to objects of size sizeof(SrcList). But in SQLite
this structure is actually allocated using:
pSrcList = malloc(sizeof(SrcList) + (nSrc-1) * sizeof(struct
SrcList_item));
where "nSrc" is the value that will be copied to pSrcList->nSrc. Thus,
although it is declared as an array of size 1, pSrcList->a[] is actually
pSrcList->nSrc elements in size.
Dan.
