> 
>         ProxyPass /uwsgi-pp uwsgi://localhost:8001/

The double '/' comes from the above, and could be avoided by using:
  ProxyPass /uwsgi-pp uwsgi://localhost:8001
or:
  ProxyPass /uwsgi-pp/ uwsgi://localhost:8001/

Using one or the other depends on whether you want e.g."/uwsgi-ppfoo" to be passed too or not (whereas "/uwsgi-pp/foo" will be passed by both).

>         ProxyPass /uwsgi-pps/ uwsgi://localhost:8001/

This one looks good.

>         ProxyPassMatch ^/admin uwsgi://localhost:8001/

Same here:
  ProxyPassMatch ^/admin uwsgi://localhost:8001
or:
  ProxyPassMatch ^/(admin/.*) uwsgi://localhost:8001/$1

> 
> I can dedicate time to work on a patch, if you have a test case for
> CVE-2021-36160 (to ensure the vulnerability stay fixed).

CVE-2021-36160 is actually fixed by r1892874, though depending on the playload it might have crashed here (we don't disclose exploits so there is no known test case).

Pointing u_path_info (PATH_INFO) to the right most leading '/' to fix your issue is an option, if you want to address it at the code level (rather than in your configuration).

I agree with Yann here. The fix to CVE-2021-36160 made a wrong configuration visible. See also http://httpd.apache.org/docs/2.4/mod/mod_proxy.html#proxypass and look for the warning there:

If the first argument ends with a trailing /, the second argument should also end with a trailing /, and vice versa. Otherwise, the resulting requests to the backend may miss some needed slashes and do not deliver the expected results. 

However, I guess the following patch would remove multiple leading slashes:

Index: modules/proxy/mod_proxy_uwsgi.c
===================================================================
--- modules/proxy/mod_proxy_uwsgi.c     (revision 1893605)
+++ modules/proxy/mod_proxy_uwsgi.c     (working copy)
@@ -481,6 +481,10 @@
                       "unable to decode uwsgi uri: %s", url);
         return HTTP_INTERNAL_SERVER_ERROR;
     }
+    /* Remove duplicate slashes at the beginning of PATH_INFO */
+    while (u_path_info[1] == '/') {
+        u_path_info++;
+    }
     apr_table_add(r->subprocess_env, "PATH_INFO", u_path_info);
 
The other question is if want to ensure that PATH_INFO at least consist of a '/'.

A request to /backend could cause an internal server error with the following configuration:

ProxyPass /backend uwsgi://localhost:8001

This would be something like this:

Index: modules/proxy/mod_proxy_uwsgi.c
===================================================================
--- modules/proxy/mod_proxy_uwsgi.c     (revision 1893605)
+++ modules/proxy/mod_proxy_uwsgi.c     (working copy)
@@ -476,11 +476,20 @@
 
     /* ADD PATH_INFO (unescaped) */
     u_path_info = ap_strchr(url + sizeof(UWSGI_SCHEME) + 2, '/');
-    if (!u_path_info || ap_unescape_url(u_path_info) != OK) {
+    if (!u_path_info) {
+        u_path_info = apr_pstrdup(r->pool, "/");
+    }
+    else if (ap_unescape_url(u_path_info) != OK) {
         ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10100)
                       "unable to decode uwsgi uri: %s", url);
         return HTTP_INTERNAL_SERVER_ERROR;
     }
+    else {
+        /* Remove duplicate slashes at the beginning of PATH_INFO */
+        while (u_path_info[1] == '/') {
+            u_path_info++;
+        }
+    }
     apr_table_add(r->subprocess_env, "PATH_INFO", u_path_info);

Hi,

Thanks for the fast answer.

I realize I didn't introduce myself clearly: I'm working on security updates for Debian LTS, and I recently issued one for the libapache2-mod-proxy-uwsgi package. I opened this bug following user reports of production failures following the security update, e.g. for
https://github.com/tracim/tracim/blob/develop/tools_docker/Debian_Uwsgi/apache2.conf.sample


> CVE-2021-36160 is actually fixed by r1892874

I'm confused, as far as I understand:
- CVE-2021-36160 is fixed by r1892805 (mod_proxy_uwsgi)
  r1892805 was shipped as a fix for CVE-2021-36160 in Debian, Ubuntu and SuSE,
  in Apache HTTPD itself, and in prior stand-alone module from unbit uwsgi
- r1892874 fixes CVE-2021-40438 (mod_proxy)
  (+ 3 follow-up commits)

The regression discussed here is caused solely by the mod_proxy_uwsgi change (r1892805).

What patch does one need to apply to fix CVE-2021-36160?


> Using one or the other depends on whether you want e.g."/uwsgi-ppfoo" to be
> passed too or not (whereas "/uwsgi-pp/foo" will be passed by both).

Slight nitpick here, I believe 'ProxyPass /uwsgi-pp' won't match '/uwsgi-ppfoo', and that the trailing slash/no-slash difference is whether '/uwsgi-pp' would be 404 or passed :)


> However, I guess the following patch would remove multiple leading slashes
> [...]
> This would be something like this:

Thanks, I confirm the second patch restores the previous behavior for my test.

Do you intend to apply this to the 2.4.x branch, or will you keep the new (stricter) behavior there?

(In reply to Sylvain Beucler from comment #3)
> 
> > CVE-2021-36160 is actually fixed by r1892874
> 
> I'm confused, as far as I understand:
> - CVE-2021-36160 is fixed by r1892805 (mod_proxy_uwsgi)
>   r1892805 was shipped as a fix for CVE-2021-36160 in Debian, Ubuntu and
> SuSE,
>   in Apache HTTPD itself, and in prior stand-alone module from unbit uwsgi
> - r1892874 fixes CVE-2021-40438 (mod_proxy)
>   (+ 3 follow-up commits)

Sorry if I wasn't clear, r1892874 and follow-up commits avoid the (known) remote exploitation of CVE-2021-36160 as well as CVE-2021-40438.
It happened that a vulnerability was reported against mod_proxy_wsgi so we fixed the flaw in mod_proxy_uwsgi (r1892805) and issued CVE-2021-36160, then further (internal-)analysis of the exploit showed that similar techniques could cause other flaws elsewhere so we fixed that in r1892874 and issued CVE-2021-40438.
Even though we couldn't find another way to crash mod_proxy_uwsgi with r1892874 applied, the initial CVE-2021-36160 was still valid should another technique be used to trigger the bug in mod_proxy_uwsgi.
Hope that helps.

> 
> The regression discussed here is caused solely by the mod_proxy_uwsgi change
> (r1892805).
> 
> What patch does one need to apply to fix CVE-2021-36160?

r1892805 will avoid the crash/DoS, but I wouldn't recommend to fix only CVE-2021-36160..

> 
> > Using one or the other depends on whether you want e.g."/uwsgi-ppfoo" to be
> > passed too or not (whereas "/uwsgi-pp/foo" will be passed by both).
> 
> Slight nitpick here, I believe 'ProxyPass /uwsgi-pp' won't match
> '/uwsgi-ppfoo', and that the trailing slash/no-slash difference is whether
> '/uwsgi-pp' would be 404 or passed :)

Ah yes, sorry for the confusion.

> 
> > However, I guess the following patch would remove multiple leading slashes
> > [...]
> > This would be something like this:
> 
> Thanks, I confirm the second patch restores the previous behavior for my
> test.
> 
> Do you intend to apply this to the 2.4.x branch, or will you keep the new
> (stricter) behavior there?

It makes sense to apply this patch to "normalize" PATH_INFO, I didn't look at other PATH_INFO usages but there might be other places where this might happen too (mod_proxy_fcgi maybe?). In any case the leading '/' is introduced by the configuration so if the application can't cope with it I'd suggest to fix that first.

Otherwise patches are always welcome ;)

Hi,

Thanks for the detailed explanation.

I understand that we need both patches (r1892805 and r1892874) to make sure CVE-2021-36160 is fixed.

> It makes sense to apply this patch to "normalize" PATH_INFO, I didn't look
> at other PATH_INFO usages but there might be other places where this might
> happen too (mod_proxy_fcgi maybe?).

AFAICT the regression reported here is caused by the mod_proxy_uwsgi change (not the mod_proxy one), so I believe other mod_proxy_* modules are not impacted.

> In any case the leading '/' is
> introduced by the configuration so if the application can't cope with it I'd
> suggest to fix that first.
> 
> Otherwise patches are always welcome ;)

I understand there's a configuration error (missing/extra slash) in the first place.

Though, we want to maintain strict backward-compatibility within a Debian long-term release. rpluem's second patch seems suitable to maintain the previous (less strict) behavior, so I'm going to apply it and I don't think additional patches are needed, but let me know if I can be of any help.

Rüdiger's patch checked in r1894074.

... and proposed for backport to 2.4.x.

(In reply to Yann Ylavic from comment #6)
> Rüdiger's patch checked in r1894074.

Thank you for pushing the patch.

In case it helps, the proposed patch was tested successfully (though not deployed in production AFAUI) by one of the Debian users who reported the issue:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=995368#48

Backported to 2.4.52.