subject:"\[sqlite\] Potential for Segmentation Violation\/Fault in sqlite 3.8.11.1"

[sqlite] Potential for Segmentation Violation/Fault in sqlite 3.8.11.1

2015-08-26 Thread Dan Kennedy

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.orig2015-08-22 18:50:01.65600 -0700
> +++ tclsqlite3.c2015-08-22 19:12:05.71600 -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*)[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   apColName[i] = Tcl_NewStringObj(sqlite3_column_name(pStmt,i), -1);
>   Tcl_IncrRefCount(apColName[i]);
> @@ -1715,6 +1736,10 @@
> zAuth = Tcl_GetStringFromObj(objv[2], );
> 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], );
> 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], );
> 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 =

[sqlite] Potential for Segmentation Violation/Fault in sqlite 3.8.11.1

2015-08-25 Thread Richard Hipp

On 8/25/15, Dan Kennedy  wrote:
> On 08/25/2015 11:36 PM, Bill Parker wrote:
>>
>> 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.
>

Sure enough.  http://tmml.sourceforge.net/doc/tcl/Alloc.html says that
you have to use Tcl_AttemptAlloc() if you want a NULL pointer returned
on OOM.  Tcl_Alloc() always panics.  See
http://core.tcl.tk/tcl/artifact/d25497d9849b8704?ln=1089 for the
implementation.

-- 
D. Richard Hipp
drh at sqlite.org

[sqlite] Potential for Segmentation Violation/Fault in sqlite 3.8.11.1

2015-08-25 Thread Bill Parker

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.orig2015-08-22 18:50:01.65600 -0700
+++ tclsqlite3.c2015-08-22 19:12:05.71600 -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*)[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; i0 ){
 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], );
   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], );
   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], );
   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], );
   if( zProfile && len>0 ){
 pDb->zProfile = Tcl_Alloc( len + 1 );
+if( !pDb->zProfile ){
+  Tcl_SetResult(interp, (char

[sqlite] Potential for Segmentation Violation/Fault in sqlite 3.8.11.1

[sqlite] Potential for Segmentation Violation/Fault in sqlite 3.8.11.1

[sqlite] Potential for Segmentation Violation/Fault in sqlite 3.8.11.1

3 matches

Site Navigation

Mail list logo

Footer information