[2.4.x] Merge r1935191, r1935192, r1935193, r1935926, r1935935, r1935941, r1936259, r1936260, r1936262, r1936263, r1936264, r1937041 from trunk

changes-entries/cern-meta-header-injection.txt

@@ -0,0 +1,2 @@
+  *) mod_cern_meta: Reject HTTP framing headers in metadata files to prevent
+     response splitting.  [Joe Orton]

changes-entries/hardening.txt

@@ -0,0 +1,4 @@
+  *) mod_cgid, mod_ssl, mod_md: Various hardening fixes.  [Joe Orton, ]
+
+  *) mod_lbmethod_heartbeat: Use safe integer parsing with range
+     validation, replacing atoi().  [Sayed Kaif <metsw24 gmail.com>]

changes-entries/interim-response-reason.txt

@@ -0,0 +1,2 @@
+  *) core: Reject control characters in the reason phrase of interim
+     responses.  Only accept space as status-code separator.  [Joe Orton]

changes-entries/substitute-maxlinelength-overflow.txt

@@ -0,0 +1,2 @@
+  *) mod_substitute: Fix SubstituteMaxLineLength to reject values too
+     large for the K/M/G suffix.  [Joe Orton]

changes-entries/substitute-pattern-oob-read.txt

@@ -0,0 +1,2 @@
+  *) mod_substitute: Fix crash or misbehaviour when loading a Substitute
+     directive with a missing closing delimiter.  [Joe Orton]

modules/filters/mod_substitute.c

@@ -627,7 +627,7 @@ static const char *set_pattern(cmd_parms *cmd, void *cfg, const char *line)
     if (delim)
         from = ++ourline;
     if (from) {
-        if (*ourline != delim) {
+        if (*ourline && *ourline != delim) {
             while (*++ourline && *ourline != delim);
         }
         if (*ourline) {
@@ -636,7 +636,7 @@ static const char *set_pattern(cmd_parms *cmd, void *cfg, const char *line)
         }
     }
     if (to) {
-        if (*ourline != delim) {
+        if (*ourline && *ourline != delim) {
             while (*++ourline && *ourline != delim);
         }
         if (*ourline) {
@@ -713,12 +713,18 @@ static const char *set_max_line_length(cmd_parms *cmd, void *cfg, const char *ar
     rv = apr_strtoff(&max, arg, &end, 10);
     if (rv == APR_SUCCESS) {
         if ((*end == 'K' || *end == 'k') && !end[1]) {
+            if (max > APR_INT64_MAX / KBYTE)
+                return "SubstituteMaxLineLength value too large";
             max *= KBYTE;
         }
         else if ((*end == 'M' || *end == 'm') && !end[1]) {
+            if (max > APR_INT64_MAX / MBYTE)
+                return "SubstituteMaxLineLength value too large";
             max *= MBYTE;
         }
         else if ((*end == 'G' || *end == 'g') && !end[1]) {
+            if (max > APR_INT64_MAX / GBYTE)
+                return "SubstituteMaxLineLength value too large";
             max *= GBYTE;
         }
         else if (*end && /* neither empty nor [Bb] */

modules/generators/mod_cgid.c

@@ -192,6 +192,8 @@ typedef struct {
 } cgid_rlimit_t;
 #endif
 
+#define ENV_COUNT_MAX (256)
+
 typedef struct {
     int req_type; /* request type (CGI_REQ, SSI_REQ, etc.) */
     unsigned long conn_id; /* connection id; daemon uses this as a hash value
@@ -201,7 +203,7 @@ typedef struct {
     pid_t ppid;            /* sanity check for config problems leading to
                             * wrong cgid socket use
                             */
-    int env_count;
+    unsigned env_count;
     ap_unix_identity_t ugid;
     apr_size_t filename_len;
     apr_size_t argv0_len;
@@ -342,7 +344,7 @@ static apr_status_t close_unix_socket(void *thefd)
 {
     int fd = (int)((long)thefd);
 
-    return close(fd);
+    return close(fd) < 0 ? errno : APR_SUCCESS;
 }
 
 /* Read from the socket dealing with incomplete messages and signals.
@@ -431,13 +433,18 @@ static apr_status_t sock_read(int fd, void *vbuf, size_t buf_size)
 static apr_status_t sock_write(int fd, const void *buf, size_t buf_size)
 {
     int rc;
+    const char *b = buf;
+    size_t written = 0;
 
     do {
-        rc = write(fd, buf, buf_size);
-    } while (rc < 0 && errno == EINTR);
-    if (rc < 0) {
-        return errno;
-    }
+        do {
+            rc = write(fd, b + written, buf_size - written);
+        } while (rc < 0 && errno == EINTR);
+        if (rc < 0) {
+            return errno;
+        }
+        written += rc;
+    } while (written < buf_size);
 
     return APR_SUCCESS;
 }
@@ -513,24 +520,30 @@ static apr_status_t get_req(int fd, request_rec *r, char **argv0, char ***env,
     if (stat != APR_SUCCESS) {
         return stat;
     }
+
+    if (req->loglevel > APLOG_TRACE8) {
+        return APR_EINVAL;
+    }
+
     r->server->log.level = req->loglevel;
     if (req->req_type == GETPID_REQ) {
         /* no more data sent for this request */
         return APR_SUCCESS;
     }
 
     /* Sanity check the structure received. */
-    if (req->env_count < 0 || req->uri_len == 0
-        || req->filename_len > APR_PATH_MAX || req->filename_len == 0
-        || req->argv0_len > APR_PATH_MAX || req->argv0_len == 0
-        || req->loglevel > APLOG_TRACE8) {
+    if (req->env_count > ENV_COUNT_MAX
+        || req->filename_len == 0 || req->filename_len > APR_PATH_MAX
+        || req->argv0_len == 0 || req->argv0_len > APR_PATH_MAX
+        || req->uri_len == 0 || req->uri_len > APR_PATH_MAX
+        || req->args_len > APR_PATH_MAX) {
         return APR_EINVAL;
     }
-    
+
     /* handle module indexes and such */
     rconf = (void **)ap_create_request_config(r->pool);
 
-    temp_core = (core_request_config *)apr_palloc(r->pool, sizeof(core_module));
+    temp_core = (core_request_config *)apr_palloc(r->pool, sizeof *temp_core);
     rconf[AP_CORE_MODULE_INDEX] = (void *)temp_core;
     r->request_config = (ap_conf_vector_t *)rconf;
     ap_set_module_config(r->request_config, &cgid_module, (void *)&req->ugid);
@@ -560,6 +573,9 @@ static apr_status_t get_req(int fd, request_rec *r, char **argv0, char ***env,
         if ((stat = sock_read(fd, &curlen, sizeof(curlen))) != APR_SUCCESS) {
             return stat;
         }
+        if (curlen > APR_PATH_MAX) {
+            return APR_EINVAL;
+        }
         environ[i] = apr_pcalloc(r->pool, curlen + 1);
         if ((stat = sock_read(fd, environ[i], curlen)) != APR_SUCCESS) {
             return stat;
@@ -862,7 +878,7 @@ static int cgid_server(void *data)
             errfileno = STDERR_FILENO;
         }
         else {
-            ap_log_error(APLOG_MARK, APLOG_DEBUG, rv, main_server,
+            ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, main_server,
                           "using passed fd %d as stderr", errfileno);
             /* Limit the received fd lifetime to pool lifetime */
             apr_pool_cleanup_register(ptrans, (void *)((long)errfileno),
@@ -1062,7 +1078,7 @@ static int cgid_init(apr_pool_t *p, apr_pool_t *plog, apr_pool_t *ptemp,
         parent_pid = getpid();
         tmp_sockname = ap_runtime_dir_relative(p, sockname);
         if (strlen(tmp_sockname) > sizeof(server_addr->sun_path) - 1) {
-            tmp_sockname[sizeof(server_addr->sun_path)] = '\0';
+            tmp_sockname[sizeof(server_addr->sun_path) - 1] = '\0';
             ap_log_error(APLOG_MARK, APLOG_ERR, 0, main_server, APLOGNO(01254)
                         "The length of the ScriptSock path exceeds maximum, "
                         "truncating to %s", tmp_sockname);
@@ -1723,8 +1739,8 @@ static void add_ssi_vars(request_rec *r)
     }
 }
 
-static int include_cmd(include_ctx_t *ctx, ap_filter_t *f,
-                       apr_bucket_brigade *bb, const char *command)
+static apr_status_t include_cmd(include_ctx_t *ctx, ap_filter_t *f,
+                                apr_bucket_brigade *bb, const char *command)
 {
     char **env;
     int sd;
@@ -1742,30 +1758,29 @@ static int include_cmd(include_ctx_t *ctx, ap_filter_t *f,
     env = ap_create_environment(r->pool, r->subprocess_env);
 
     if ((retval = connect_to_daemon(&sd, r, conf)) != OK) {
-        return retval;
+        return APR_EGENERAL;
     }
 
-    send_req(sd, NULL, r, command, env, SSI_REQ);
+    rv = send_req(sd, NULL, r, command, env, SSI_REQ);
+    if (rv) {
+        ap_log_rerror(APLOG_MARK, APLOG_DEBUG, rv, r,
+                      "could not send request to cgi daemon (for SSI)");
+        return rv;
+    }
 
     info = apr_palloc(r->pool, sizeof(struct cleanup_script_info));
     info->conf = conf;
     info->r = r;
     rv = get_cgi_pid(r, conf, &(info->pid));
-    if (APR_SUCCESS == rv) {             
-        /* for this type of request, the script is invoked through an
-         * intermediate shell process...  cleanup_script is only able
-         * to knock out the shell process, not the actual script
-         */
-        apr_pool_cleanup_register(r->pool, info,
-                                  cleanup_script,
-                                  apr_pool_cleanup_null);
-    }
-    else { 
-        ap_log_rerror(APLOG_MARK, APLOG_DEBUG, rv, r, "error determining cgi PID (for SSI)");
+    if (rv) {
+        ap_log_rerror(APLOG_MARK, APLOG_DEBUG, rv, r, "error determining cgi daemon PID (for SSI)");
+        return rv;
     }
 
-    apr_pool_cleanup_register(r->pool, info,
-                              cleanup_script,
+    /* For this type of request, the script is invoked through an
+     * intermediate shell process...  cleanup_script is only able to
+     * knock out the shell process, not the actual script. */
+    apr_pool_cleanup_register(r->pool, info, cleanup_script,
                               apr_pool_cleanup_null);
 
     /* We are putting the socket discriptor into an apr_file_t so that we can

modules/http/http_filters.c

@@ -1001,7 +1001,7 @@ static apr_status_t validate_status_line(request_rec *r)
         if (len < 3
             || apr_strtoi64(r->status_line, &end, 10) != r->status
             || (end - 3) != r->status_line
-            || (len >= 4 && ! apr_isspace(r->status_line[3]))) {
+            || (len >= 4 && r->status_line[3] != ' ')) {
             r->status_line = NULL;
             return APR_EGENERAL;
         }

modules/md/md_crypt.c

@@ -2226,7 +2226,7 @@ apr_status_t md_cert_get_ari_cert_id(const char **pari_cert_id,
     const ASN1_INTEGER *serial;
     BIGNUM *bn;
     int i = -1, sder_len;
-    unsigned char *ucp, sbuf[256];
+    unsigned char *ucp, *sbuf;
 
     *pari_cert_id = NULL;
     s_aki = X509_get_ext_d2i(cert->x509, NID_authority_key_identifier, &i, NULL);
@@ -2253,6 +2253,10 @@ apr_status_t md_cert_get_ari_cert_id(const char **pari_cert_id,
     }
     memset(&ser_buf, 0, sizeof(ser_buf));
     bn = ASN1_INTEGER_to_BN(serial, NULL);
+    if (!bn) {
+        return APR_EINVAL;
+    }
+    sbuf = apr_pcalloc(p, BN_num_bytes(bn));
     sder_len = BN_bn2bin(bn, sbuf);
     BN_free(bn);
     if (sder_len < 1)

modules/metadata/mod_cern_meta.c

@@ -256,6 +256,18 @@ static int scan_meta_file(request_rec *r, apr_file_t *f)
             sscanf(l, "%d", &r->status);
             r->status_line = apr_pstrdup(r->pool, l);
         }
+        else if (!ap_cstr_casecmp(w, "Transfer-Encoding")
+                 || !ap_cstr_casecmp(w, "Content-Length")
+                 || !ap_cstr_casecmp(w, "Connection")
+                 || !ap_cstr_casecmp(w, "Trailer")
+                 || !ap_cstr_casecmp(w, "Upgrade")
+                 || !ap_cstr_casecmp(w, "Keep-Alive")
+                 || !ap_cstr_casecmp(w, "TE")) {
+            ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10596)
+                          "forbidden HTTP framing header '%s' in meta file: %s",
+                          w, r->filename);
+            return HTTP_INTERNAL_SERVER_ERROR;
+        }
         else {
             apr_table_set(tmp_headers, w, l);
         }

modules/proxy/balancers/mod_lbmethod_heartbeat.c

@@ -61,6 +61,28 @@ typedef struct ctx_servers {
     apr_hash_t *servers;
 } ctx_servers_t;
 
+static int hb_parse_int(const char *val, int min, int max, int *result)
+{
+    apr_int64_t parsed;
+    char *end = NULL;
+
+    if (!val || !*val) {
+        return 0;
+    }
+
+    errno = 0;
+    parsed = apr_strtoi64(val, &end, 10);
+    if (errno == ERANGE || end == val || *end != '\0') {
+        return 0;
+    }
+    if (parsed < min || parsed > max) {
+        return 0;
+    }
+
+    *result = (int)parsed;
+    return 1;
+}
+
 static void
 argstr_to_table(apr_pool_t *p, char *str, apr_table_t *parms)
 {
@@ -179,19 +201,31 @@ static apr_status_t readfile_heartbeats(const char *path, apr_hash_t *servers,
             argstr_to_table(pool, apr_pstrdup(pool, t), hbt);
 
             if ((val = apr_table_get(hbt, "busy"))) {
-                server->busy = atoi(val);
+                int parsed;
+                if (hb_parse_int(val, 0, INT_MAX, &parsed)) {
+                    server->busy = parsed;
+                }
             }
 
             if ((val = apr_table_get(hbt, "ready"))) {
-                server->ready = atoi(val);
+                int parsed;
+                if (hb_parse_int(val, 0, INT_MAX, &parsed)) {
+                    server->ready = parsed;
+                }
             }
 
             if ((val = apr_table_get(hbt, "lastseen"))) {
-                server->seen = atoi(val);
+                int parsed;
+                if (hb_parse_int(val, 0, INT_MAX, &parsed)) {
+                    server->seen = parsed;
+                }
             }
 
             if ((val = apr_table_get(hbt, "port"))) {
-                server->port = atoi(val);
+                int parsed;
+                if (hb_parse_int(val, 1, 65535, &parsed)) {
+                    server->port = parsed;
+                }
             }
 
             if (server->busy == 0 && server->ready != 0) {
@@ -312,7 +346,13 @@ static proxy_worker *find_best_hb(proxy_balancer *balancer,
         if (PROXY_WORKER_IS_USABLE(*worker)) {
             server->worker = *worker;
             if (server->seen < LBM_HEARTBEAT_MAX_LASTSEEN) {
-                openslots += server->ready;
+                apr_uint32_t ready = (apr_uint32_t)server->ready;
+                if (ready > APR_UINT32_MAX - openslots) {
+                    openslots = APR_UINT32_MAX;
+                }
+                else {
+                    openslots += ready;
+                }
                 APR_ARRAY_PUSH(up_servers, hb_server_t *) = server;
             }
         }
@@ -325,12 +365,20 @@ static proxy_worker *find_best_hb(proxy_balancer *balancer,
         pick = ap_random_pick(0, openslots);
 
         for (i = 0; i < up_servers->nelts; i++) {
+            apr_uint32_t upper;
             server = APR_ARRAY_IDX(up_servers, i, hb_server_t *);
-            if (pick >= c && pick <= c + server->ready) {
+            if ((apr_uint32_t)server->ready > APR_UINT32_MAX - c) {
+                upper = APR_UINT32_MAX;
+            }
+            else {
+                upper = c + (apr_uint32_t)server->ready;
+            }
+
+            if (pick >= c && pick <= upper) {
                 mycandidate = server->worker;
             }
 
-            c += server->ready;
+            c = upper;
         }
     }
 

modules/ssl/ssl_engine_kernel.c

@@ -2195,7 +2195,7 @@ static apr_status_t set_challenge_creds(conn_rec *c, const char *servername,
 cleanup:
     if (our_data && cert) X509_free(cert);
     if (our_data && key) EVP_PKEY_free(key);
-    return APR_SUCCESS;
+    return rv;
 }
 
 /*

modules/ssl/ssl_engine_ocsp.c

@@ -80,7 +80,7 @@ static apr_uri_t *determine_responder_uri(SSLSrvConfigRec *sc, X509 *cert,
     }
 
     rv = apr_uri_parse(p, s, u);
-    if (rv || !u->hostname) {
+    if (rv || !u->hostname || !u->scheme) {
         ap_log_cerror(APLOG_MARK, APLOG_DEBUG, rv, c, APLOGNO(01919)
                       "failed to parse OCSP responder URI '%s'", s);
         return NULL;