Mailing-List: contact commits-help@apr.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@apr.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1079951 - in /apr/apr-util/branches/1.3.x: CHANGES
 dbd/apr_dbd_odbc.c
Date: Wed, 09 Mar 2011 19:20:35 -0000
To: commits@apr.apache.org
From: trawick@apache.org
Message-Id: <20110309192035.7CA8123888BD@eris.apache.org>

Author: trawick
Date: Wed Mar  9 19:20:35 2011
New Revision: 1079951

URL: http://svn.apache.org/viewvc?rev=1079951&view=rev
Log:
merge from apr trunk:

r1078737:
fix some existing parameter range checking that had a bad
assumption about the size of array entries

add the same range check in another path

r1078744:
replace some strcpy() calls with apr_cpystrn()
(no overflow currently)

r1079872:
zap the never-used apr_env_get() retcode

r1079901:
apr_dbd odbc: Fix stack buffer overwrite when an unexpected
number of parameters is passed to open.


Modified:
    apr/apr-util/branches/1.3.x/CHANGES
    apr/apr-util/branches/1.3.x/dbd/apr_dbd_odbc.c

Modified: apr/apr-util/branches/1.3.x/CHANGES
URL: http://svn.apache.org/viewvc/apr/apr-util/branches/1.3.x/CHANGES?rev=1079951&r1=1079950&r2=1079951&view=diff
==============================================================================
--- apr/apr-util/branches/1.3.x/CHANGES [utf-8] (original)
+++ apr/apr-util/branches/1.3.x/CHANGES [utf-8] Wed Mar  9 19:20:35 2011
@@ -1,6 +1,10 @@
                                                      -*- coding: utf-8 -*-
 Changes with APR-util 1.3.11
 
+  *) DBD ODBC support: Fix stack buffer overwrite when an unexpected
+     number of parameters is passed to open.  Fix range checking of the
+     APR DBD type enum passed to some of the APIs.  [Jeff Trawick]
+
   *) Add support for Berkeley DB 5.1.
      [Rainer Jung]
 

Modified: apr/apr-util/branches/1.3.x/dbd/apr_dbd_odbc.c
URL: http://svn.apache.org/viewvc/apr/apr-util/branches/1.3.x/dbd/apr_dbd_odbc.c?rev=1079951&r1=1079950&r2=1079951&view=diff
==============================================================================
--- apr/apr-util/branches/1.3.x/dbd/apr_dbd_odbc.c (original)
+++ apr/apr-util/branches/1.3.x/dbd/apr_dbd_odbc.c Wed Mar  9 19:20:35 2011
@@ -202,7 +202,7 @@ typedef struct {
 
 /* SQL datatype mappings to DBD datatypes 
  * These tables must correspond *exactly* to the apr_dbd_type_e enum 
- * in apr_dbd_internal.h 
+ * in apr_dbd.h
  */
 
 /* ODBC "C" types to DBD datatypes  */
@@ -231,6 +231,7 @@ static SQLSMALLINT const sqlCtype[] = {
     SQL_LONGVARCHAR,                /* APR_DBD_TYPE_CLOB,       \%pDc */
     SQL_TYPE_NULL                   /* APR_DBD_TYPE_NULL        \%pDn */
 };
+#define NUM_APR_DBD_TYPES (sizeof(sqlCtype) / sizeof(sqlCtype[0]))
 
 /*  ODBC Base types to DBD datatypes */
 static SQLSMALLINT const sqlBaseType[] = {
@@ -528,6 +529,10 @@ static SQLRETURN odbc_bind_param(apr_poo
     }
     /* bind a non-NULL data value */
     else {
+        if (type < 0 || type >= NUM_APR_DBD_TYPES) {
+            return APR_EGENERAL;
+        }
+
         baseType = sqlBaseType[type];
         cType = sqlCtype[type];
         indicator = NULL;
@@ -814,7 +819,7 @@ static apr_status_t odbc_parse_params(ap
                                int *defaultBufferSize, int *nattrs,
                                int **attrs, int **attrvals)
 {
-    char *seps, *last, *name[MAX_PARAMS], *val[MAX_PARAMS];
+    char *seps, *last, *next, *name[MAX_PARAMS], *val[MAX_PARAMS];
     int nparams = 0, i, j;
 
     *attrs = apr_pcalloc(pool, MAX_PARAMS * sizeof(char *));
@@ -835,8 +840,18 @@ static apr_status_t odbc_parse_params(ap
         }
         val[nparams] = apr_strtok(NULL, seps, &last);
         seps = DEFAULTSEPS;
-        name[++nparams] = apr_strtok(NULL, seps, &last);
-    } while (nparams <= MAX_PARAMS && name[nparams] != NULL);
+
+        ++nparams;
+        next = apr_strtok(NULL, seps, &last);
+        if (!next) {
+            break;
+        }
+        if (nparams >= MAX_PARAMS) {
+            /* too many parameters, no place to store */
+            return APR_EGENERAL;
+        }
+        name[nparams] = next;
+    } while (1);
 
     for (j = i = 0; i < nparams; i++) {
         if (!apr_strnatcasecmp(name[i], "CONNECT")) {
@@ -905,15 +920,16 @@ static void check_error(apr_dbd_t *dbc, 
     SQLSMALLINT reslength;
     char *res, *p, *end, *logval = NULL;
     int i;
-    apr_status_t r;
 
     /* set info about last error in dbc  - fast return for SQL_SUCCESS  */
     if (rc == SQL_SUCCESS) {
         char successMsg[] = "[dbd_odbc] SQL_SUCCESS ";
+        apr_size_t successMsgLen = sizeof successMsg - 1;
 
         dbc->lasterrorcode = SQL_SUCCESS;
-        strcpy(dbc->lastError, successMsg);
-        strcpy(dbc->lastError + sizeof(successMsg) - 1, step);
+        apr_cpystrn(dbc->lastError, successMsg, sizeof dbc->lastError);
+        apr_cpystrn(dbc->lastError + successMsgLen, step,
+                    sizeof dbc->lastError - successMsgLen);
         return;
     }
     switch (rc) {
@@ -954,7 +970,7 @@ static void check_error(apr_dbd_t *dbc, 
         if (SQL_SUCCEEDED(rc) && (p < (end - 280))) 
             p += sprintf(p, "%.256s %.20s ", buffer, sqlstate);
     }
-    r = apr_env_get(&logval, "apr_dbd_odbc_log", dbc->pool);
+    apr_env_get(&logval, "apr_dbd_odbc_log", dbc->pool);
     /* if env var was set or call was init/open (no dbname) - log to stderr */
     if (logval || !dbc->dbname ) {
         char timestamp[APR_CTIME_LEN];
@@ -970,7 +986,8 @@ static APR_INLINE int odbc_check_rollbac
 {
     if (handle->can_commit == APR_DBD_TRANSACTION_ROLLBACK) {
         handle->lasterrorcode = SQL_ERROR;
-        strcpy(handle->lastError, "[dbd_odbc] Rollback pending ");
+        apr_cpystrn(handle->lastError, "[dbd_odbc] Rollback pending ",
+                    sizeof handle->lastError);
         return 1;
     }
     return 0;
@@ -1339,15 +1356,17 @@ static apr_status_t odbc_datum_get(const
 {
     SQLSMALLINT sqltype;
     void *p;
-    int len = sqlSizes[dbdtype];
+    int len;
 
     if (col >= row->res->ncols)
         return APR_EGENERAL;
 
-    if (dbdtype < 0 || dbdtype >= sizeof(sqlCtype)) {
+    if (dbdtype < 0 || dbdtype >= NUM_APR_DBD_TYPES) {
         data = NULL;            /* invalid type */
         return APR_EGENERAL;
     }
+
+    len = sqlSizes[dbdtype];
     sqltype = sqlCtype[dbdtype];
 
     /* must not memcpy a brigade, sentinals are relative to orig loc */