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:
=======================================================================
--- 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)
