Merge r1663647, r1679181, r1679182 from trunk:

* mod_authn_dbd: apr_pstrdup dbd_password and dbd_hash to fix use-after-free
bug with postgresql

mod_authn_dbd, mod_authz_dbd, mod_session_dbd, mod_rewrite: Fix lifetime
of DB lookup entries independently of the selected DB engine.  PR 46421.

Suggested by: Michel Stam <michel reverze net>
Proposed by: Steven whitson <steven.whitson gmail com>
Reviewed/Extended/Committed by: ylavic

Follup up to r1679181: CHANGES entry.
Submitted by: jkaluza, ylavic, ylavic
Reviewed/backported by: jim

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1681107 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index 1b69efde5f713e92d151b5940419421927368eef..f759fd87d451e7c2853c919b30d935c4ad1fbef3 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -12,6 +12,10 @@ Changes with Apache 2.4.13
      calls r:wsupgrade() can cause a child process crash. 
      [Edward Lu <Chaosed0 gmail.com>]
 
+  *) mod_authn_dbd, mod_authz_dbd, mod_session_dbd, mod_rewrite: Fix lifetime
+     of DB lookup entries independently of the selected DB engine.  PR 46421.
+     [Steven whitson <steven.whitson gmail com>, Jan Kaluza, Yann Ylavic].
+
   *) In alignment with RFC 7525, the default recommended SSLCipherSuite
      and SSLProxyCipherSuite now exclude RC4 as well as MD5. Also, the
      default recommended SSLProtocol and SSLProxyProtocol directives now

diff --git a/STATUS b/STATUS
index 2d69669330e574fc2f51a669ca96e089558bb732..efbe05fc6fe32e09edbd805d70140e0e71d769a4 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -105,13 +105,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
-  *) mod_authn_dbd, mod_authz_dbd, mod_session_dbd, mod_rewrite: Fix lifetime
-     of DB lookup entries independently of the selected DB engine.  PR 46421.
-     trunk patch: http://svn.apache.org/r1663647
-                  http://svn.apache.org/r1679181
-                  http://svn.apache.org/r1679182
-     2.4.x patch: trunk works (modulo CHANGES)
-     +1: ylavic, minfrin, jkaluza, wrowe
 
 
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:

diff --git a/modules/aaa/mod_authn_dbd.c b/modules/aaa/mod_authn_dbd.c
index 45c3dc3c7f0cdce2c508d99ade0a196a827196f2..b9bd3739ae2eeebf013a1ce16412849d265724e0 100644 (file)
--- a/modules/aaa/mod_authn_dbd.c
+++ b/modules/aaa/mod_authn_dbd.c
@@ -174,7 +174,8 @@ static authn_status authn_dbd_password(request_rec *r, const char *user,
                 i++;
             }
 #endif
-            dbd_password = apr_dbd_get_entry(dbd->driver, row, 0);
+            dbd_password = apr_pstrdup(r->pool,
+                                       apr_dbd_get_entry(dbd->driver, row, 0));
         }
         /* we can't break out here or row won't get cleaned up */
     }
@@ -269,7 +270,8 @@ static authn_status authn_dbd_realm(request_rec *r, const char *user,
                 i++;
             }
 #endif
-            dbd_hash = apr_dbd_get_entry(dbd->driver, row, 0);
+            dbd_hash = apr_pstrdup(r->pool,
+                                   apr_dbd_get_entry(dbd->driver, row, 0));
         }
         /* we can't break out here or row won't get cleaned up */
     }

diff --git a/modules/aaa/mod_authz_dbd.c b/modules/aaa/mod_authz_dbd.c
index 30749914a7d7778e4479f79cf4cb2585240144fd..a165eb0ec91c460dd2c310f464c93efb407d37d7 100644 (file)
--- a/modules/aaa/mod_authz_dbd.c
+++ b/modules/aaa/mod_authz_dbd.c
@@ -174,7 +174,9 @@ static int authz_dbd_login(request_rec *r, authz_dbd_cfg *cfg,
                           action, r->user, message?message:noerror);
                 }
                 else if (newuri == NULL) {
-                    newuri = apr_dbd_get_entry(dbd->driver, row, 0);
+                    newuri =
+                        apr_pstrdup(r->pool,
+                                    apr_dbd_get_entry(dbd->driver, row, 0));
                 }
                 /* we can't break out here or row won't get cleaned up */
             }
@@ -204,7 +206,6 @@ static int authz_dbd_group_query(request_rec *r, authz_dbd_cfg *cfg,
     apr_dbd_prepared_t *query;
     apr_dbd_results_t *res = NULL;
     apr_dbd_row_t *row = NULL;
-    const char **group;
 
     if (cfg->query == NULL) {
         ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01649)
@@ -224,8 +225,9 @@ static int authz_dbd_group_query(request_rec *r, authz_dbd_cfg *cfg,
              rv != -1;
              rv = apr_dbd_get_row(dbd->driver, r->pool, res, &row, -1)) {
             if (rv == 0) {
-                group = apr_array_push(groups);
-                *group = apr_dbd_get_entry(dbd->driver, row, 0);
+                APR_ARRAY_PUSH(groups, const char *) =
+                    apr_pstrdup(r->pool,
+                                apr_dbd_get_entry(dbd->driver, row, 0));
             }
             else {
                 message = apr_dbd_error(dbd->driver, dbd->handle, rv);

diff --git a/modules/mappers/mod_rewrite.c b/modules/mappers/mod_rewrite.c
index a879f6a0c91f3be7072c6cc54fd2a8cdb373bc7f..dfacdaea8d701cdb84714ac91081f3a05ff7ebd1 100644 (file)
--- a/modules/mappers/mod_rewrite.c
+++ b/modules/mappers/mod_rewrite.c
@@ -1352,12 +1352,14 @@ static char *lookup_map_dbd(request_rec *r, char *key, const char *label)
     while ((rv = apr_dbd_get_row(db->driver, r->pool, res, &row, -1)) == 0) {
         ++n;
         if (ret == NULL) {
-            ret = apr_dbd_get_entry(db->driver, row, 0);
+            ret = apr_pstrdup(r->pool,
+                              apr_dbd_get_entry(db->driver, row, 0));
         }
         else {
             /* randomise crudely amongst multiple results */
             if ((double)rand() < (double)RAND_MAX/(double)n) {
-                ret = apr_dbd_get_entry(db->driver, row, 0);
+                ret = apr_pstrdup(r->pool,
+                                  apr_dbd_get_entry(db->driver, row, 0));
             }
         }
     }
@@ -1370,11 +1372,11 @@ static char *lookup_map_dbd(request_rec *r, char *key, const char *label)
     case 0:
         return NULL;
     case 1:
-        return apr_pstrdup(r->pool, ret);
+        return ret;
     default:
         /* what's a fair rewritelog level for this? */
         rewritelog((r, 3, NULL, "Multiple values found for %s", key));
-        return apr_pstrdup(r->pool, ret);
+        return ret;
     }
 }
 

diff --git a/modules/session/mod_session_dbd.c b/modules/session/mod_session_dbd.c
index a6ab40ea6f573b8d4915e64a7f2eaf6674e31612..cf65e5af3cdb54b19bec9f0ad783224143e1f05c 100644 (file)
--- a/modules/session/mod_session_dbd.c
+++ b/modules/session/mod_session_dbd.c
@@ -138,7 +138,8 @@ static apr_status_t dbd_load(request_rec * r, const char *key, const char **val)
             return APR_EGENERAL;
         }
         if (*val == NULL) {
-            *val = apr_dbd_get_entry(dbd->driver, row, 0);
+            *val = apr_pstrdup(r->pool,
+                               apr_dbd_get_entry(dbd->driver, row, 0));
         }
         /* we can't break out here or row won't get cleaned up */
     }