Possible NULL DEREFERENCES and DEAD STORES found by static analysis tools

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Possible NULL DEREFERENCES and DEAD STORES found by static analysis tools

Patricia Monteiro
I have been analyzing the latest version of SQLite (3.24.0) with several
static analysis tools (Infer, Clang Static Analyzer, Cppcheck and Predator)
and after manually reviewing the code I have identified the following
errors:

1)
Location: sqlite3.c: 91920
Error: NULL DEREFERENCE
Found by: Clang Static Analyzer
Description: In the vdbeMergeEngineInit function of the sqlite3.c: 91920
file, the * pMerger! = NULL check is not performed before doing the
dereference.

2)
Location: sqlite3.c: 199054
Type: NULL DEREFERENCE
Found by: Clang Static Analyzer
Description: In the function fts5BufferCompare arguments are passed to the
memcmp function without checking if they are! = NULL.

3)
Location: sqlite3.c: 169136
Type: NULL DEREFERENCE
Found by: Clang Static Analyzer
Description: In the fts3LcsIteratorAdvance function, the * pIter! = NULL
check is not performed before doing the dereference.

4)
Location: sqlite3.c: 130155
Type: NULL DEREFERENCE
Found by: Clang Static Analyzer
Description: In the sqlite3UpsertDoUpdate function, a situation similar to
the previous one occurs, ie you need to check if pUpsert! = NULL.

5)
Location: sqlite3.c: 128851 / sqlite3.c: 113375 / sqlite3.c: 113253 /
sqlite3.c: 112108 / sqlite3.c: 112344 / sqlite3.c: 106296 / sqlite3.c:
104629
Type: NULL DEREFERENCE
Found by: Infer
Description: The sqlite3GetVdbe function may return NULL, so you must check
if v! = NULL before using it as an argument.

6)
Location: shell.c: 3329
Type: DEAD STORE
Tool: Infer
Description: The iArg counter is incremented at line 3329, however its
value is no longer used in the completionFilter function.
The error report returned by Infer is:

shell.c:3329: error: DEAD_STORE

  The value written to &iArg (type int) is never used.

  3327.         if( pCur->zLine==0 ) return SQLITE_NOMEM;

  3328.       }

  3329. >     iArg++;

  3330.     }

  3331.     if( pCur->zLine!=0 && pCur->zPrefix==0 ){

7)
Location: shell.c: 13799 / shellc.c: 14953 /shell.c:4877 /sqlite3.c:67330
Type: DEAD STORE
Found by: Infer and Clang Static Analyzer
Description: The value written in rc is not used, a new value is written to
the variable without the previously written value being used.
One of the error reports returned by Infer  is:

shell.c:14953: error: DEAD_STORE

  The value written to &rc (type int) is never used.

  14951.         }

  14952.       }

  14953. >     rc = sqlite3_finalize(pStmt);

  14954.       appendText(&s, " ORDER BY 1", 0);

  14955.       rc = sqlite3_prepare_v2(p->db, s.z, -1, &pStmt, 0);

One of the bug reports returned by Clang Static Analyzerzer is:

shell.c:14953:5: warning: Value stored to 'rc' is never read

    rc = sqlite3_finalize(pStmt);

    ^    ~~~~~~~~~~~~~~~~~~~~~~~
8)
Location: sqlite3.c: 120122
Type: DEAD STORE
Found by: Infer and Clang Static Analyzer
Description: The value written to the cSep variable is never used. This
variable is used within a for loop on line 120116, however outside that
loop is not used again after being assigned the value ',' at line 120122.
Infer's results report is:

sqlite3.c:120122: error: DEAD_STORE

  The value written to &cSep (type char) is never used.

  120120.     if( i==0 ){

  120121.       sqlite3_str_appendf(&acc, "(\"%s\"", pPragma->zName);

  120122. >     cSep = ',';

  120123.       i++;

  120124.     }

9)
Location: sqlite3.c: 135444
Type: DEAD STORE
Found by: Infer and Clang Static Analyzer
Description: The value written to the pTerm variable is never used.
Infer's results report is:

sqlite3.c:135444: error: DEAD_STORE

  The value written to &pTerm (type WhereTerm*) is never used.

  135442.           testcase( idxNew==0 );

  135443.           exprAnalyze(pSrc, pWC, idxNew);

  135444. >         pTerm = &pWC->a[idxTerm];

  135445.           markTermAsChild(pWC, idxNew, idxTerm);

  135446.         }else{


10)
Location: sqlite3.c: 75150
Type: DEAD STORE
Found by: Infer and Clang Static Analyzer
Description: The variable * v is initialized at line 75150 with the value
pParse-> pVdbe, which is not used and at line 75156 is assigned to * v the
same value.
Code:

SQLITE_PRIVATE void sqlite3VdbeExplain(Parse *pParse, u8 bPush, const char
*zFmt, ...){

  if( pParse->explain==2 ){

    char *zMsg;

    Vdbe *v = pParse->pVdbe;

    va_list ap;

    int iThis;

    va_start(ap, zFmt);

    zMsg = sqlite3VMPrintf(pParse->db, zFmt, ap);

    va_end(ap);

    v = pParse->pVdbe;

    iThis = v->nOp;

    sqlite3VdbeAddOp4(v, OP_Explain, iThis, pParse->addrExplain, 0,

                      zMsg, P4_DYNAMIC);

    if( bPush) pParse->addrExplain = iThis;

  }

}

Infer's results report is:

sqlite3.c:75150: error: DEAD_STORE

  The value written to &v (type Vdbe*) is never used.

  75148.     if( pParse->explain==2 ){

  75149.       char *zMsg;

  75150. >     Vdbe *v = pParse->pVdbe;

  75151.       va_list ap;

  75152.       int iThis;
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Possible NULL DEREFERENCES and DEAD STORES found by static analysis tools

Niall O'Reilly
On 21 Aug 2018, at 10:14, Patricia Monteiro wrote:

> I have been analyzing the latest version of SQLite (3.24.0) with several
> static analysis tools (Infer, Clang Static Analyzer, Cppcheck and Predator)
> and after manually reviewing the code I have identified the following
> errors:

Variants of this question crop up from time to time.

Please look in the mailing-list archives for replies from Richard Hipp dated
22 January 2014 and 23 March 2015, sent in response to earlier similar reports.

Best regards,

Niall O'Reilly



_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users

signature.asc (921 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Possible NULL DEREFERENCES and DEAD STORES found by static analysis tools

wmertens
I was curious so I  looked it up, the 2015 one is here
http://sqlite.1065341.n5.nabble.com/Security-issues-in-SQLite-td81339.html
but the 2014 one didn't get any replies.

The gist of it is that these static analysis tools generate a lot of false
positives, so unless you can come up with a test case where the condition
is triggered, it's probably not a real issue...

On Tue, Aug 21, 2018, 7:53 PM Niall O'Reilly <[hidden email]> wrote:

> On 21 Aug 2018, at 10:14, Patricia Monteiro wrote:
>
> > I have been analyzing the latest version of SQLite (3.24.0) with several
> > static analysis tools (Infer, Clang Static Analyzer, Cppcheck and
> Predator)
> > and after manually reviewing the code I have identified the following
> > errors:
>
> Variants of this question crop up from time to time.
>
> Please look in the mailing-list archives for replies from Richard Hipp
> dated
> 22 January 2014 and 23 March 2015, sent in response to earlier similar
> reports.
>
> Best regards,
>
> Niall O'Reilly
>
>
> _______________________________________________
> sqlite-users mailing list
> [hidden email]
> http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
>
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users