Bug 64598 - mod_http2 and mod_proxy_uwsgi: segfault in uwsgi_send_headers()
Summary: mod_http2 and mod_proxy_uwsgi: segfault in uwsgi_send_headers()
Status: NEW
Alias: None
Product: Apache httpd-2
Classification: Unclassified
Component: mod_http2 (show other bugs)
Version: 2.4.43
Hardware: PC Linux
: P2 normal (vote)
Target Milestone: ---
Assignee: Apache HTTPD Bugs Mailing List
URL:
Keywords: FixedInTrunk, PatchAvailable
Depends on:
Blocks:
 
Reported: 2020-07-14 08:25 UTC by Petr Gajdos
Modified: 2020-07-15 08:25 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Petr Gajdos 2020-07-14 08:25:29 UTC
Original bug report including backtrace:
https://bugzilla.suse.com/show_bug.cgi?id=1174052

The easiest way to reproduce:

$ git clone https://github.com/pgajdos/apache-rex.git
$ cd apache-rex
$ HTTPD_MPM=worker ./run-rex mod_proxy_uwsgi-http2
[..]
$ cat /tmp/apache-rex/mod_proxy_uwsgi-http2/error_log
[Tue Jul 14 10:18:32.584670 2020] [mpm_worker:notice] [pid 670:tid 140293301127168] AH00292: Apache/2.4.43 (Linux/SUSE) OpenSSL/1.1.1g configured -- resuming normal operations
[Tue Jul 14 10:18:32.584841 2020] [core:notice] [pid 670:tid 140293301127168] AH00094: Command line: 'httpd -f /tmp/apache-rex/mod_proxy_uwsgi-http2/httpd.conf'
[Tue Jul 14 10:18:37.395546 2020] [core:notice] [pid 670:tid 140293301127168] AH00051: child pid 674 exit signal Segmentation fault (11), possible coredump in /srv/www
[Tue Jul 14 10:18:37.412147 2020] [mpm_worker:notice] [pid 670:tid 140293301127168] AH00295: caught SIGTERM, shutting down
$

I can work out testcase which does not need apache-rex framework if you wish and of course help other way.
Comment 1 Petr Gajdos 2020-07-14 09:49:20 UTC
(gdb) frame 1
#1  0x00007ffff4838e4f in uwsgi_send_headers (r=0x55555599ac20, conn=0x55555599ec40) at mod_proxy_uwsgi.c:178
178	        headerlen += 2 + strlen(env[j].key) + 2 + strlen(env[j].val);
(gdb) p env[j]
$6 = {key = 0x7ffff59d78d8 "H2_STREAM_ID", val = 0x0, key_checksum = 1209163603}
(gdb)
Comment 2 Ruediger Pluem 2020-07-14 09:56:27 UTC
Does the below patch fix your issue?

Index: modules/proxy/mod_proxy_uwsgi.c
===================================================================
--- modules/proxy/mod_proxy_uwsgi.c	(revision 1879840)
+++ modules/proxy/mod_proxy_uwsgi.c	(working copy)
@@ -175,7 +175,7 @@
     env = (apr_table_entry_t *) env_table->elts;
 
     for (j = 0; j < env_table->nelts; ++j) {
-        headerlen += 2 + strlen(env[j].key) + 2 + strlen(env[j].val);
+        headerlen += 2 + strlen(env[j].key) + 2 + env[j].val ? strlen(env[j].val) : 0;
     }
 
     ptr = buf = apr_palloc(r->pool, headerlen);
@@ -189,10 +189,12 @@
         memcpy(ptr, env[j].key, keylen);
         ptr += keylen;
 
-        vallen = strlen(env[j].val);
+        vallen = env[j].val ? strlen(env[j].val) : 0;
         *ptr++ = (apr_byte_t) (vallen & 0xff);
         *ptr++ = (apr_byte_t) ((vallen >> 8) & 0xff);
-        memcpy(ptr, env[j].val, vallen);
+        if (env[j].val) {
+            memcpy(ptr, env[j].val, vallen);
+        }
         ptr += vallen;
     }
Comment 4 Petr Gajdos 2020-07-14 09:57:48 UTC
(In reply to Ruediger Pluem from comment #2)
> Does the below patch fix your issue?
> 
> Index: modules/proxy/mod_proxy_uwsgi.c
> ===================================================================
> --- modules/proxy/mod_proxy_uwsgi.c	(revision 1879840)
> +++ modules/proxy/mod_proxy_uwsgi.c	(working copy)
> @@ -175,7 +175,7 @@
>      env = (apr_table_entry_t *) env_table->elts;
>  
>      for (j = 0; j < env_table->nelts; ++j) {
> -        headerlen += 2 + strlen(env[j].key) + 2 + strlen(env[j].val);
> +        headerlen += 2 + strlen(env[j].key) + 2 + env[j].val ?
> strlen(env[j].val) : 0;
>      }
>  
>      ptr = buf = apr_palloc(r->pool, headerlen);
> @@ -189,10 +189,12 @@
>          memcpy(ptr, env[j].key, keylen);
>          ptr += keylen;
>  
> -        vallen = strlen(env[j].val);
> +        vallen = env[j].val ? strlen(env[j].val) : 0;
>          *ptr++ = (apr_byte_t) (vallen & 0xff);
>          *ptr++ = (apr_byte_t) ((vallen >> 8) & 0xff);
> -        memcpy(ptr, env[j].val, vallen);
> +        if (env[j].val) {
> +            memcpy(ptr, env[j].val, vallen);
> +        }
>          ptr += vallen;
>      }

Will check ..
Comment 5 Petr Gajdos 2020-07-14 10:53:28 UTC
You probably wanted
 headerlen += 2 + strlen(env[j].key) + 2 + (env[j].val ? strlen(env[j].val) : 0;
right?

Now it does not crash, yes. I will ask original reporter for testing.
Comment 6 Petr Gajdos 2020-07-14 11:10:04 UTC
(In reply to Petr Gajdos from comment #5)
> You probably wanted
>  headerlen += 2 + strlen(env[j].key) + 2 + (env[j].val ? strlen(env[j].val)
> : 0;

Eh,
headerlen += 2 + strlen(env[j].key) + 2 + (env[j].val ? strlen(env[j].val  : 0);
of course.
Comment 7 Petr Gajdos 2020-07-14 11:12:42 UTC
Heh, I actually mean
headerlen += 2 + strlen(env[j].key) + 2 + (env[j].val ? strlen(env[j].val) : 0);
Comment 8 Ruediger Pluem 2020-07-14 12:13:18 UTC
(In reply to Petr Gajdos from comment #7)
> Heh, I actually mean
> headerlen += 2 + strlen(env[j].key) + 2 + (env[j].val ? strlen(env[j].val) :
> 0);

That's what I meant :-)
Comment 9 Petr Gajdos 2020-07-15 06:39:06 UTC
The reporter says everything works.

Thanks Ruediger for very very fast response!
Comment 10 Ruediger Pluem 2020-07-15 08:25:06 UTC
Committed to trunk as r1879878.