On 08/25/2015 11:36 PM, Bill Parker wrote:
> Hello All,
>
> In reviewing source code files in sqlite 3.8.11.1, I found some
> instances of calls to Tcl_Alloc() which are not checked for a return
> value of NULL, indicating failure in directory '/tea/generic', file
> 'tclsqlite3.c'. Additionally, in the event of failure, there are
> some cases where memset()/memcpy() is called after Tcl_Alloc(), but
> in the event that Tcl_Alloc() returns NULL, memset()/memcpy() will
> generate a segmentation fault/violation if memset()/memcpy() is called
> with a address location pointing to NULL (see test program below
> the patch file).
>
> The patch file below should catch and handle all conditions where
> Tcl_Alloc() is called, but are NOT checked for a return value of NULL:
Does Tcl_Alloc() actually return NULL if a malloc fails? I thought if
memory can not be allocated it calls Tcl_Panic() to report an error
message and then aborts the process.
Dan.
>
> =======================================================================
>
> --- tclsqlite3.c.orig 2015-08-22 18:50:01.656000000 -0700
> +++ tclsqlite3.c 2015-08-22 19:12:05.716000000 -0700
> @@ -380,6 +380,10 @@
> }
>
> p = (IncrblobChannel *)Tcl_Alloc(sizeof(IncrblobChannel));
> + if( !p ){
> + Tcl_SetResult(interp, (char *)"malloc failed", TCL_STATIC);
> + return TCL_ERROR;
> + }
> p->iSeek = 0;
> p->pBlob = pBlob;
>
> @@ -439,6 +443,10 @@
> SqlFunc *p, *pNew;
> int nName = strlen30(zName);
> pNew = (SqlFunc*)Tcl_Alloc( sizeof(*pNew) + nName + 1 );
> + if( !pNew ){
> + Tcl_SetResult(interp, (char *)"malloc failed", TCL_STATIC);
> + return NULL; /* what should be returned here? */
> + }
> pNew->zName = (char*)&pNew[1];
> memcpy(pNew->zName, zName, nName+1);
> for(p=pDb->pFunc; p; p=p->pNext){
> @@ -1168,6 +1176,10 @@
> nVar = sqlite3_bind_parameter_count(pStmt);
> nByte = sizeof(SqlPreparedStmt) + nVar*sizeof(Tcl_Obj *);
> pPreStmt = (SqlPreparedStmt*)Tcl_Alloc(nByte);
> + if( !pPreStmt ){
> + Tcl_SetResult(interp, (char *)"malloc failed", TCL_STATIC);
> + return TCL_ERROR;
> + }
> memset(pPreStmt, 0, nByte);
>
> pPreStmt->pStmt = pStmt;
> @@ -1177,6 +1189,11 @@
> #ifdef SQLITE_TEST
> if( pPreStmt->zSql==0 ){
> char *zCopy = Tcl_Alloc(pPreStmt->nSql + 1);
> + if( !zCopy ) {
> + Tcl_SetResult(interp, (char *)"malloc failed", TCL_STATIC);
> + Tcl_Free(pPreStmt);
> + return TCL_ERROR;
> + }
> memcpy(zCopy, zSql, pPreStmt->nSql);
> zCopy[pPreStmt->nSql] = '\0';
> pPreStmt->zSql = zCopy;
> @@ -1372,6 +1389,10 @@
> p->nCol = nCol = sqlite3_column_count(pStmt);
> if( nCol>0 && (papColName || p->pArray) ){
> apColName = (Tcl_Obj**)Tcl_Alloc( sizeof(Tcl_Obj*)*nCol );
> + if( !apColName ){
> + Tcl_SetResult(interp, (char *)"malloc failed", TCL_STATIC);
> + return;
> + }
> for(i=0; i<nCol; i++){
> apColName[i] = Tcl_NewStringObj(sqlite3_column_name(pStmt,i), -1);
> Tcl_IncrRefCount(apColName[i]);
> @@ -1715,6 +1736,10 @@
> zAuth = Tcl_GetStringFromObj(objv[2], &len);
> if( zAuth && len>0 ){
> pDb->zAuth = Tcl_Alloc( len + 1 );
> + if( !pDb->zAuth ){
> + Tcl_SetResult(interp, (char *)"malloc failed", TCL_STATIC);
> + return TCL_ERROR;
> + }
> memcpy(pDb->zAuth, zAuth, len+1);
> }else{
> pDb->zAuth = 0;
> @@ -1804,6 +1829,10 @@
> zBusy = Tcl_GetStringFromObj(objv[2], &len);
> if( zBusy && len>0 ){
> pDb->zBusy = Tcl_Alloc( len + 1 );
> + if( !pDb->zBusy ){
> + Tcl_SetResult(interp, (char *)"malloc failed", TCL_STATIC);
> + return TCL_ERROR;
> + }
> memcpy(pDb->zBusy, zBusy, len+1);
> }else{
> pDb->zBusy = 0;
> @@ -1970,6 +1999,10 @@
> zCommit = Tcl_GetStringFromObj(objv[2], &len);
> if( zCommit && len>0 ){
> pDb->zCommit = Tcl_Alloc( len + 1 );
> + if( !pDb->zCommit ){
> + Tcl_SetResult(interp, (char *)"malloc failed", TCL_STATIC);
> + return TCL_ERROR;
> + }
> memcpy(pDb->zCommit, zCommit, len+1);
> }else{
> pDb->zCommit = 0;
> @@ -2315,6 +2348,10 @@
> Tcl_IncrRefCount(pScript);
>
> p = (DbEvalContext *)Tcl_Alloc(sizeof(DbEvalContext));
> + if( !p ){
> + Tcl_SetResult(interp, (char *)"malloc failed", TCL_STATIC);
> + return TCL_ERROR;
> + }
> dbEvalInit(p, pDb, objv[2], pArray);
>
> cd2[0] = (void *)p;
> @@ -2458,6 +2495,10 @@
> }
> if( zNull && len>0 ){
> pDb->zNull = Tcl_Alloc( len + 1 );
> + if( !pDb->zNULL ){
> + Tcl_SetResult(interp, (char *)"malloc failed", TCL_STATIC);
> + return TCL_ERROR;
> + }
> memcpy(pDb->zNull, zNull, len);
> pDb->zNull[len] = '\0';
> }else{
> @@ -2513,6 +2554,10 @@
> zProgress = Tcl_GetStringFromObj(objv[3], &len);
> if( zProgress && len>0 ){
> pDb->zProgress = Tcl_Alloc( len + 1 );
> + if( !pDb->zProgress ){
> + Tcl_SetResult(interp, (char *)"malloc failed", TCL_STATIC);
> + return TCL_ERROR;
> + }
> memcpy(pDb->zProgress, zProgress, len+1);
> }else{
> pDb->zProgress = 0;
> @@ -2555,6 +2600,10 @@
> zProfile = Tcl_GetStringFromObj(objv[2], &len);
> if( zProfile && len>0 ){
> pDb->zProfile = Tcl_Alloc( len + 1 );
> + if( !pDb->zProfile ){
> + Tcl_SetResult(interp, (char *)"malloc failed", TCL_STATIC);
> + return TCL_ERROR;
> + }
> memcpy(pDb->zProfile, zProfile, len+1);
> }else{
> pDb->zProfile = 0;
> @@ -2741,6 +2790,10 @@
> zTrace = Tcl_GetStringFromObj(objv[2], &len);
> if( zTrace && len>0 ){
> pDb->zTrace = Tcl_Alloc( len + 1 );
> + if( !pDb->zTrace ){
> + Tcl_SetResult(interp, (char *)"malloc failed", TCL_STATIC);
> + return TCL_ERROR;
> + }
> memcpy(pDb->zTrace, zTrace, len+1);
> }else{
> pDb->zTrace = 0;
>
>
> =======================================================================
>
> Here is a test program which shows the potential issue with memcpy()
> and a NULL address location (compiled with: gcc -O2 -Wall -g memcpy.c
>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
>
> int main(void)
> {
> char *buf;
> int x = 10;
>
> buf = malloc(80);
> #if 1
> buf = NULL; /* force buf to point to NULL */
> #endif
>
> while (0 <-- x)
> printf("Value of x is: %d\n", x);
>
> printf("before memcpy() is called...\n");
> memcpy(buf, "hello", 5); /* what happens? */
> printf("after memcpy() is called...\n");
>
> printf("Address of buf is: %p, value of buf is: %s\n", buf, buf);
>
> return 0;
> }
>
> Here is the output from 'a.out' (Fedora 22 Server, gcc 5.1)
>
> [bill at moocow ~]$ ./a.out
> Value of x is: 9
> Value of x is: 8
> Value of x is: 7
> Value of x is: 6
> Value of x is: 5
> Value of x is: 4
> Value of x is: 3
> Value of x is: 2
> Value of x is: 1
> before memcpy() is called...
> Segmentation fault (core dumped)
>
> Here is the same program with the buf = NULL statement
> commented out via the pre-processor:
>
> [bill at moocow ~]$ cat memcpy-works.c
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
>
> int main(void)
> {
> char *buf;
> int x = 10;
>
> buf = malloc(80);
> #if 0
> buf = NULL; /* force buf to point to NULL */
> #endif
>
> while (0 <-- x)
> printf("Value of x is: %d\n", x);
>
> printf("before memcpy() is called...\n");
> memcpy(buf, "hello", 5); /* what happens? */
> printf("after memcpy() is called...\n");
>
> printf("Address of buf is: %p, value of buf is: %s\n", buf, buf);
>
> return 0;
> }
>
> Here is the output from ./a.out:
>
> [bill at moocow ~]$ ./a.out
> Value of x is: 9
> Value of x is: 8
> Value of x is: 7
> Value of x is: 6
> Value of x is: 5
> Value of x is: 4
> Value of x is: 3
> Value of x is: 2
> Value of x is: 1
> before memcpy() is called...
> after memcpy() is called...
> Address of buf is: 0x24df010, value of buf is: hello
>
>
> I am attaching the patch file to this bug report...
>
> Questions, Comments, Suggestions, Complaints? :)
>
> Bill Parker (wp02855 at gmail dot com)
>
>
> _______________________________________________
> sqlite-users mailing list
> sqlite-users at mailinglists.sqlite.org
> http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
