Page MenuHomePhabricator

Apache 2.4.61 throws a 403 Forbidden for links containing %3F
Closed, ResolvedPublicSecurity

Description

Summarizing this from a WMF Slack conversation.

The Web Task Creation Form linked at https://www.mediawiki.org/wiki/Reading/Web/Request_process throws a 403.

Narrowing this down, so does the following:

https://phabricator.wikimedia.org/maniphest/task/edit/form/1/?title=%3F

From /var/log/apache2/phabricator_error.log:

[Mon Jul 15 20:38:16.826622 2024] [rewrite:error] [pid 947281:tid 947294] [client redacted] AH10508: Unsafe URL with %3f URL rewritten without UnsafeAllow3F

This led us to a Stackoverflow question (403 Forbidden when URL contains GET with encoded question mark / UnsafeAllow3F / rawurlencode), and thus to CVE-2024-38474:

Substitution encoding issue in mod_rewrite in Apache HTTP Server 2.4.59 and earlier allows attacker to execute scripts in directories permitted by the configuration but not directly reachable by any URL or source disclosure of scripts meant to only to be executed as CGI. Users are recommended to upgrade to version 2.4.60, which fixes this issue. Some RewriteRules that capture and substitute unsafely will now fail unless rewrite flag "UnsafeAllow3F" is specified.

Upstream patch:

https://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/mappers/mod_rewrite.c?r1=1918560&r2=1918561&pathrev=1918561&diff_format=h

Some verbatim discussion:

I only know of this affecting phabricator.wikimedia.org, but it seems possible it might come up elsewhere. Initially filing this as a security task out of a probable excess of caution.

Event Timeline

@bd808: The RewriteRule ^(.*)$ /index.php?__path__=$1 [B,L,QSA] rule at https://gerrit.wikimedia.org/r/plugins/gitiles/operations/puppet/+/refs/heads/production/modules/phabricator/templates/phabricator-default.conf.erb#75 is going to trigger on this. Not sure what else.

This actually might not be the trigger point? The B flag here should keep the %3F in the inbound URL from being expanded. As I understand the bug & fix Apache is now checking to see if there was a %3F in the inbound URL and a ? in the redirect target and refusing to continue when this is the case.

The new stop logic happens right after rewritelog((r, 2, ctx->perdir, "rewrite '%s' -> '%s'", ctx->uri, newuri));. If we turn up the LogLevel on a Phab node to trace2 I think we should see that before/after and maybe figure out which rule should really have a B flag added.

If we turn up the LogLevel on a Phab node to trace2 I think we should see that before/after and maybe figure out which rule should really have a B flag added.

Could use devtools' https://phabricator.wmcloud.org/ for testing here.

If we turn up the LogLevel on a Phab node to trace2 I think we should see that before/after and maybe figure out which rule should really have a B flag added.

Could use devtools' https://phabricator.wmcloud.org/ for testing here.

trace2 output with https://phabricator.wmcloud.org/maniphest/task/edit/form/1/?title=%3F

[Mon Jul 15 23:34:51.252461 2024] [rewrite:trace2] [pid 1641369:tid 1641590] mod_rewrite.c(504): [client 172.16.5.238:51504] 172.16.5.238 - - [phabricator.wmcloud.org/sid#7f44fe7f81d8][rid#7f44fe6cf0a0/initial] init rewrite engine with requested uri /maniphest/task/edit/form/1/
[Mon Jul 15 23:34:51.252578 2024] [rewrite:trace2] [pid 1641369:tid 1641590] mod_rewrite.c(504): [client 172.16.5.238:51504] 172.16.5.238 - - [phabricator.wmcloud.org/sid#7f44fe7f81d8][rid#7f44fe6cf0a0/initial] rewrite '/maniphest/task/edit/form/1/' -> '/index.php?__path__=%2fmaniphest%2ftask%2fedit%2fform%2f1%2f'
[Mon Jul 15 23:34:51.252602 2024] [rewrite:error] [pid 1641369:tid 1641590] [client 172.16.5.238:51504] AH10508: Unsafe URL with %3f URL rewritten without UnsafeAllow3F
[Mon Jul 15 23:34:51.252622 2024] [rewrite:trace2] [pid 1641369:tid 1641590] mod_rewrite.c(504): [client 172.16.5.238:51504] 172.16.5.238 - - [phabricator.wmcloud.org/sid#7f44fe7f81d8][rid#7f44fe6cf0a0/initial] setting lastsub to rule with output /index.php?__path__=$1

rewrite '/maniphest/task/edit/form/1/' -> '/index.php?__path__=%2fmaniphest%2ftask%2fedit%2fform%2f1%2f'

I think this actually shows us that the upstream patch is buggy. The ? that is being matched by the guard condition is one we put in ourselves and not an expansion of the %2f in the inbound URL.

@bd808: The RewriteRule ^(.*)$ /index.php?__path__=$1 [B,L,QSA] rule at https://gerrit.wikimedia.org/r/plugins/gitiles/operations/puppet/+/refs/heads/production/modules/phabricator/templates/phabricator-default.conf.erb#75 is going to trigger on this. Not sure what else.

This actually might not be the trigger point? The B flag here should keep the %3F in the inbound URL from being expanded. As I understand the bug & fix Apache is now checking to see if there was a %3F in the inbound URL and a ? in the redirect target and refusing to continue when this is the case.

Logs do indeed point to that line as the culprit, two things from trace5:

  1. Unsure if it's applying backreference to the query string vs. just passing it on as is. (I don't see rewritelog((ctx->r, 5, ctx->perdir, "escaping backreference '%s' to '%s'",tmp,tmp2)); entries for the query string)
  2. The url https://phabricator.wmcloud.org/test/%3f goes through:
mod_rewrite.c(504): escaping backreference '/test/?' to '%2ftest%2f%3f'
mod_rewrite.c(504): rewrite '/test/?' -> '/index.php?__path__=%2ftest%2f%3f'
[rewrite:error]  AH10508: Unsafe URL with %3f URL rewritten without UnsafeAllow3F

If we're sure about the maniphests/edit endpoint handling %3f correctly, but don't want to open UnsafeAllow3f for everything we could:

RewriteRule ^(/maniphest/task/edit/.*)$ /index.php?__path__=$1 [B,L,QSA,UnsafeAllow3F]
RewriteRule ^(.*)$          /index.php?__path__=$1  [B,L,QSA]

Tentatively that got upgraded on July 12th at 9:17:

/var/log/apt/history.log
Start-Date: 2024-07-12  09:17:00
Commandline: apt-get -q -y -o DPkg::Options::=--force-confold -o DPkg::Options::=--force-confdef install apache2=2.4.61-1~deb11u1 apache2-bin=2.4.61-1~deb11u1 apache2-data=2.4.61-1~deb11u1 apache2-utils=2.4.61-1~deb11u1
Upgrade: apache2-data:amd64 (2.4.59-1~deb11u1, 2.4.61-1~deb11u1), apache2-bin:amd64 (2.4.59-1~deb11u1, 2.4.61-1~deb11u1), apache2-utils:amd64 (2.4.59-1~deb11u1, 2.4.61-1~deb11u1), apache2:amd64 (2.4.59-1~deb11u1, 2.4.61-1~deb11u1)
End-Date: 2024-07-12  09:17:09

Which restarted the apache2.service:

Active: active (running) since Fri 2024-07-12 09:17:07 UTC; 4 days ago
Jul 12 00:00:25 phab1004 systemd[1]: Reloaded The Apache HTTP Server.
Jul 12 09:17:02 phab1004 systemd[1]: Stopping The Apache HTTP Server...
Jul 12 09:17:07 phab1004 systemd[1]: apache2.service: Succeeded.
Jul 12 09:17:07 phab1004 systemd[1]: Stopped The Apache HTTP Server.
Jul 12 09:17:07 phab1004 systemd[1]: apache2.service: Consumed 18h 32min 24.077s CPU time.
Jul 12 09:17:07 phab1004 systemd[1]: Starting The Apache HTTP Server...
Jul 12 09:17:07 phab1004 systemd[1]: Started The Apache HTTP Server.
Jul 13 00:00:26 phab1004 systemd[1]: Reloading The Apache HTTP Server.

I don't know how it got upgraded though since nobody and nothing logged in at that time?

brennen  pts/0        2620:0:861:4:208 Mon Jul 15 20:36 - 22:02  (01:26)
dzahn    pts/0        2620:0:861:4:208 Fri Jul 12 23:34 - 23:40  (00:05)
aklapper pts/0        2a02:ec80:600:1: Thu Jul 11 23:32 - 23:40  (00:07)
jelto    pts/0        2620:0:861:4:208 Thu Jul 11 08:42 - 08:59  (00:16)
aklapper pts/0        2a02:ec80:600:1: Wed Jul 10 17:48 - 18:24  (00:35)

Eventually i found from the sshd log there was a connection from Cumin:

Jul 12 09:16:57 phab1004 sshd[3342510]: Connection from 2620:0:861:107:10:64:48:98 port 42282 on 2620:0:861:102:10:64:16:101 port 22 rdomain ""
Jul 12 09:16:58 phab1004 sshd[3342510]: Accepted key x y z found at /etc/ssh/userkeys/root.d/cumin:1

That points me to {T369885} which was a mass update of Apache 2 on all Bullseye / Bookworm hosts.

The Apache error number is`AH10508` and I guess it might affect more services.

For the few services sending Apache logs to OpenSearch, we can find the errors via https://logstash.wikimedia.org/app/dashboards#/view/apache2log and it shows doc1003 is also affected. Then the message does not give the requested or rewritten URL. On the Apache access log dashboard. I have found two requests for https://doc.wikimedia.org/oojs-ui/master/demos/?page=icons&theme=wikimediaui&direction=ltr&platform=desktop which gave a 403, then I don't why they'd be rewritten or why they would give a 403. So those are a mystery.

I don't think MediaWiki had the update applied since it is stuck to Buster for now, and I imagine it might be affected :-/

If we're sure about the maniphests/edit endpoint handling %3f correctly, but don't want to open UnsafeAllow3f for everything we could:

RewriteRule ^(/maniphest/task/edit/.*)$ /index.php?__path__=$1 [B,L,QSA,UnsafeAllow3F]
RewriteRule ^(.*)$          /index.php?__path__=$1  [B,L,QSA]

Using the UnsafeAllow3F flag on the existing /index.php?... rule seems like a reasonable fix to me:

RewriteRule ^(.*)$          /index.php?__path__=$1  [B,L,QSA,UnsafeAllow3F]

We can see in the logging from T370110#9983690 that the url encoded %3F values are passing through as expected and the ? that is triggering the error is actually the one we explicitly are adding to support the front router setup of Phorge's index.php entry point.

Using the UnsafeAllow3F flag on the existing /index.php?... rule seems like a reasonable fix to me:

RewriteRule ^(.*)$          /index.php?__path__=$1  [B,L,QSA,UnsafeAllow3F]

We can see in the logging from T370110#9983690 that the url encoded %3F values are passing through as expected and the ? that is triggering the error is actually the one we explicitly are adding to support the front router setup of Phorge's index.php entry point.

👍

RewriteRule ^(.*)$          /index.php?__path__=$1  [B,L,QSA,UnsafeAllow3F]

Is that something to file in Phorge upstream to fix https://we.phorge.it/book/phorge/article/configuration_guide/ in src/docs/user/configuration/configuration_guide.diviner ? If yes, should that be a non-public upstream ticket?

brennen triaged this task as High priority.Jul 17 2024, 6:15 PM

Is that something to file in Phorge upstream to fix https://we.phorge.it/book/phorge/article/configuration_guide/ in src/docs/user/configuration/configuration_guide.diviner ? If yes, should that be a non-public upstream ticket?

I think yes and... Maybe for the moment?

testing with httpbb from deploy1002.

with a test yaml file with content:

https://phabricator.wikimedia.org:
- path: /maniphest/task/edit/form/default/?title=%3f
  assert_status: 403

It's 403 on both machines before the patch:

[deploy1002:~] $ httpbb ./test_phabricator.yaml --hosts=phab2002.codfw.wmnet
Sending to phab2002.codfw.wmnet...
PASS: 1 request sent to phab2002.codfw.wmnet. All assertions passed.

disabled puppet on phab* hosts.

manually applied patch on phab2002 only

but it becomes a 500 with the patch:

[deploy1002:~] $ httpbb ./test_phabricator.yaml --hosts=phab2002.codfw.wmnet
Sending to phab2002.codfw.wmnet...
https://phabricator.wikimedia.org/maniphest/task/edit/form/default/?title=%3f (./test_phabricator.yaml:2)
    Status code: expected 403, got 500.

and I don't see this in error logs so far...

This is what happens when I request path: /maniphest/task/edit/form/default/?title=%3f

It becomes url.full":"http://phabricator.wikimedia.org/maniphest/task/edit/form/default/?__path__=%2fmaniphest%2ftask%2fedit%2fform%2fdefault%2f&title=%3f", with a 500.

==> /var/log/apache2/phabricator_access.log <==
@cee: {"client.ip":"2620:0:860:103:10:192:32:54", "ecs.version":"1.7.0", "event.category":["network", "web"], "event.dataset":"apache.access", "event.duration":"94383", "event.kind":"event", "event.outcome":"unknown", "event.type":["access", "connection"], "http.request.headers.accept_language":"-", "http.request.headers.x_analytics":"-", "http.request.headers.x_client_ip":"-", "http.request.headers.x_forwarded_for":"-", "http.request.headers.x_request_id":"8cc536b3-7086-4465-b8f6-4b064be76e5e", "http.request.method":"GET", "http.request.referrer":"-", "http.response.bytes":"1348", "http.response.headers.content_type":"text/html", "http.response.headers.user":"-", "http.response.status_code":"500", "labels.handler":"proxy:unix:/run/php/fpm-www.sock|fcgi://localhost/", "labels.httpd_server_name":"phabricator.wikimedia.org", "server.ip":"2620:0:860:103:10:192:32:54", "service.type":"apache2", "source.ip":"2620:0:860:103:10:192:32:54", "timestamp":"2024-07-17T18:46:31", "url.domain":"phabricator.wikimedia.org", "url.full":"http://phabricator.wikimedia.org/maniphest/task/edit/form/default/?__path__=%2fmaniphest%2ftask%2fedit%2fform%2fdefault%2f&title=%3f", "url.path":"/maniphest/task/edit/form/default/", "url.query":"?__path__=%2fmaniphest%2ftask%2fedit%2fform%2fdefault%2f&title=%3f", "user.name":"-", "user_agent.original":"httpbb (https://wikitech.wikimedia.org/wiki/Httpbb) dzahn@deploy1002"}
19:04 <mutante> curl -svk --resolve phabricator.wikimedia.org:443:10.64.16.101 'https://phabricator.wikimedia.org/maniphest/task/edit/form/1/?title=%3F'
19:04 <mutante> this is the IP of phab1004 (no patch) .. and it's 403 Forbidden indeed

19:04 <mutante> curl -svk --resolve phabricator.wikimedia.org:443:10.192.32.54 'https://phabricator.wikimedia.org/maniphest/task/edit/form/1/?title=%3F'
19:05 <mutante> thi sis the IP of phab2002 (patched) and it's:   <body class="setup-fatal">
13:36 <brennen> ok so i think what's happening here is this:
13:36 <brennen> 1. without the patch, apache throws the 403 for the rewrite rule before phab ever sees it
13:37 <brennen> 2. after the patch, the rewrite rule is applied as intended and the traffic hits phabricator
13:37 <brennen> 3. but phab2002 is always going to return that error page
13:38 <brennen> so i think this can't easily be tested on phab2002

I think getting the error page is actually an indicator that the patch resolves the issue, since that's what I'd normally expect for other URLs:

21:10:04 brennen@deploy1002 ~ $ curl -sk --resolve phabricator.wikimedia.org:443:10.192.32.54 'https://phabricator.wikimedia.org/' | tail
}

.phui-two-column-view .setup-issue-shell {
  width: auto;
  margin: 0;
}
</style>
  </head>
  <body class="setup-fatal"><div class="setup-issue-shell"><div class="setup-issue"><div class="setup-issue-head"><div class="setup-issue-name">Replicating Master</div></div><div class="setup-issue-body"><div class="setup-issue-instructions">Database host &quot;m3-slave.codfw.wmnet:3323&quot; is configured as a master, but is replicating another host. This is dangerous and can mangle or destroy data. Only replicas should be replicating. Stop replication on the host or adjust configuration.</div></div><div class="setup-issue-tail"></div></div><div class="setup-issue-next">To continue, resolve this problem and reload the page.</div><div class="setup-issue-debug">Host: phab2002</div></div></body>
brennen lowered the priority of this task from High to Medium.Jul 17 2024, 11:47 PM

Looks like this is resolved for phab with the merge of https://gerrit.wikimedia.org/r/c/operations/puppet/+/1054907 - leaving this open in case the issue surfaces elsewhere.

I confirm what Brennen said above.

I manually edited config and tested the patch on phab1004 with httpbb. I got 200 there.

phab2002: before 403 after 500
phab1004: before 403 after 200

Then deployed the puppet change and it was a noop, same thing as the manual change before.

[deploy1002:~] $ cat test_phabricator.yaml 
https://phabricator.wikimedia.org:
- path: /maniphest/task/edit/form/default/?title=%3f
  assert_status: 200


[deploy1002:~] $ httpbb ./test_phabricator.yaml --hosts=phab1004.eqiad.wmnet
Sending to phab1004.eqiad.wmnet...
PASS: 1 request sent to phab1004.eqiad.wmnet. All assertions passed.

Also adding the test to httpbb https://gerrit.wikimedia.org/r/c/operations/puppet/+/1054956

Should this task remain non-public? Asking as I'd like to upstream but unsure about the level of caution.
https://gerrit.wikimedia.org/r/c/operations/puppet/+/1054907 is public...

Mstyles changed the visibility from "Custom Policy" to "Public (No Login Required)".Jul 22 2024, 4:32 PM
Mstyles changed the edit policy from "Custom Policy" to "All Users".

@Aklapper since the gerrit patch is public this ticket is okay to be public as well. I went ahead and changed the policy

Is the B flag the reason the issue triggers? From what I understand it encodes the requested URI before it is processed and surely any legit ones having a question mark will end up triggering it. Maybe the upstream code should have exempted those cases, then I don't understand the attack vector :-/ What I am wondering is what is the sufficient condition to trigger the error so that we can audit all of our RewriteRule.

For MediaWiki on Kubernetes Apache configuration, the httpd-cfgi image has two config at https://gerrit.wikimedia.org/g/operations/docker-images/production-images/+/refs/heads/master/images/httpd-fcgi/config/sites/ with the following RewriteRule statements:

000-default.conf:    RewriteRule ^/index.php$ - [L]
000-default.conf:    RewriteRule ^/static/.* - [L]
000-default.conf:    RewriteRule ^.*$ /index.php [L]
999-monitoring.conf:    RewriteRule ^/(metrics|healthz)$ /index.php [L]
999-monitoring.conf:    RewriteRule ^/(server-status|fpm-status|livez)$ - [L]
999-monitoring.conf:    RewriteRule . - [R=404]

Which I guess are not affected since they don't mention a backreference (eg $1) in the rewritten portion.

If I look at upstream patch it has:

if (!(p->flags & RULEFLAG_UNSAFE_ALLOW3F) &&
    ap_strcasestr(r->unparsed_uri, "%3f") &&
    ap_strchr_c(newuri, '?')) {
    ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO()
            "Unsafe URL with %%3f URL rewritten without "
            "UnsafeAllow3F");
    r->status = HTTP_FORBIDDEN;
    return RULE_RC_STATUS_SET;
}

Thus that would be any RewriteRule having a capture group with a new URL having a question mark. A capture group would have an opening parenthesis .*\(.+? and the rewritten URL has a question mark .*\?.*?. From Puppet code:

$ git grep -P 'RewriteRule .*\(.+? .*\?.*?'
hieradata/common/mediawiki.yaml:              - "    RewriteRule ^/wiki/Special:Code(Review)? %%{}{ENV:RW_PROTO}://static-codereview.wikimedia.org? [R=301,L]"
hieradata/common/mediawiki.yaml:              - "RewriteRule ^/wikihole(/)?$ https://donate.wikimedia.org/?utm_medium=podcast&utm_campaign=wikihole&utm_source=wikihole [R=302,NE,L]"
modules/phabricator/templates/phabricator-default.conf.erb:  RewriteRule ^(.*)$          /index.php?__path__=$1  [B,L,QSA,UnsafeAllow3F]
modules/phabricator/templates/phabricator-local.conf.erb:  RewriteRule ^(.*)$          /index.php?__path__=$1  [B,L,QSA,UnsafeAllow3F]
modules/profile/templates/phorge/httpd.conf.erb:  RewriteRule ^(.*)$    /index.php?__path__=$1  [B,L,QSA]

The open questions:

  • is the grep correct and thus is that an exhaustive list?
  • are the two redirects for MediaWiki an issue? Given:
    • If the Special:CodeReview redirect is broken, that is not the end of the world
    • The https//donate.wikipedia.org/wikihole/ redirect is flagged for removal in 2026 (T357436) but should meanwhile still be supported. Though I guess it is unlikely to receive a request with a %3F.
  • The phorge module in Puppet should be adjusted

Change #1056207 had a related patch set uploaded (by Dzahn; author: Dzahn):

[operations/puppet@production] phorge: add UnsafeAllow3F rewrite flag

https://gerrit.wikimedia.org/r/1056207

Change #1056207 merged by Dzahn:

[operations/puppet@production] phorge: add UnsafeAllow3F rewrite flag

https://gerrit.wikimedia.org/r/1056207

  • The phorge module in Puppet should be adjusted

Done

  • The phorge module in Puppet should be adjusted

Done

I forgot to send the patch :D Danke Schon.

As for whether MediaWiki or something else is affected, I have no idea, but it looks closely tied to the use of the B flag.

I'm tempted to resolve this task tagged with Phabricator and create a separate one for MediaWiki-Vagrant (is the modules/profile/templates/idp/client/httpd-puppetboard-ng.erb result in Puppet relevant?) because I dislike fixed issues displayed as unresolved tasks in my backlog. Eh?

modules/profile/templates/idp/client/httpd-puppetboard-ng.erb is for https://puppetboard.wikimedia.org

That would be neither Phabricator nor Vagrant.

If this has bitten us anywhere else, I'm not aware of it. I think it seems fine to resolve at this stage.

@Aklapper are you okay to resolve this ticket?

See the previous three comments

brennen claimed this task.