fastcgi: Possible Arbitrary Code Execution with Null Bytes, PHP, Windows

[MisterDuval]

Step to reproduce sent at Matt + Francis
This character is not RFC compliant and should be avoid in all paths

[francislavoie]

I agree that this is probably a fine workaround for the case you found, but on the other hand it feels like a band-aid because we don't truly understand why the PHP upstream breaks in that way. Without understanding that, we can't truly know this is actually a fix or just hiding a problem.

I haven't been able to reproduce it myself (because frankly as a volunteer I don't have the time or will to set up a fresh Windows VM to attempt to do so).

But what we do know, Go's URL-decoding seems to also decode %00 sequences as well in the net/url package, and it feels like it probably shouldn't. I think it should be brought upstream to the Go team, either for them to fix something, or to tell us what we're doing wrong, if we are.

See this section of the URI RFC, which seems relevant here: https://datatracker.ietf.org/doc/html/rfc3986#section-7.3

[MisterDuval]

I do agree it can hide other problem, but we should fix this flaw in the time we find a full workaround.
This flaw is huge, and let the hacker get very important Windows files, we need to react.

[francislavoie]

I disagree on the assessment of severity. We can't really judge how severe it is without understand why it happens upstream, like I said.

We know it only happens when there's no index.php in the webroot (that alone is pretty rare) and it only happens on some versions of Windows (for reasons still unknown). The way I see it, this only affects a fraction of a fraction of a fraction of users.

[mholt]

And it only reproduces on Windows 10 Pro, not Home?

I'm also conflicted as to whether this is actually a bug in the server and not the backend application. My understanding of this behavior is that the server is just passing along the path, and the backend is interpreting it, which is where the problem emerges.

[MisterDuval]

And it only reproduces on Windows 10 Pro, not Home?

Only Pro OS', so all servers versions, windows 7,10,11 Pro... Not on Home OS'.

[MisterDuval]

I'm also conflicted as to whether this is actually a bug in the server and not the backend application. My understanding of this behavior is that the server is just passing along the path, and the backend is interpreting it, which is where the problem emerges.

I agree, but Nginx responds a "400 Bad Request". I'm pretty sure this %00 character should be denied first on Caddy's side.

[MisterDuval]

@francislavoie why is this not happening on Caddy 1 ? The reverseproxy part seems to be the same...

[mholt]

@MisterDuval

Only Pro OS', so all servers versions, windows 7,10,11 Pro... Not on Home OS'.

That is so, so weird. Do we know why? I want to understand this better before plunging in.

I agree, but Nginx responds a "400 Bad Request". I'm pretty sure this %00 character should be denied first on Caddy's side.

That might be true, although, I'm still torn on whether that is a semantic or boundary violation (like, trying to assume how URLs will be interpreted without actually needing to interpret them). We've gotten in trouble with stuff like this in the past and usually we've found it best to just not meddle with things, especially fundamental things at the core of HTTP servers.

why is this not happening on Caddy 1 ? The reverseproxy part seems to be the same...

They aren't the same. Caddy 2 is a complete rewrite from scratch.

[MisterDuval]

That is so, so weird. Do we know why? I want to understand this better before plunging in.

I don't, I just catched this because there was a difference between my and Francis laptops and my desktop at work + servers in prod.

That might be true, although, I'm still torn on whether that is a semantic or boundary violation (like, trying to assume how URLs will be interpreted without actually needing to interpret them). We've gotten in trouble with stuff like this in the past and usually we've found it best to just not meddle with things, especially fundamental things at the core of HTTP servers.

I understand, but as this %00 sign is not allowed in URLs, as RFC tells, we should better block it before passing it to backends which can bring flaws like this one. Maybe it's not Caddy's code, but it leads to a flaw. Nginx blocks it for a good reason... This was a known flaw on Nginx's versions before 2011.
My problem is that we cannot even block the %00 request with a Caddy configuration, if not fixed in core, as this character is not matchable (from my tests, maybe you could try to?).

why is this not happening on Caddy 1 ? The reverseproxy part seems to be the same...

I looked at the code where I patched the flaw, it was pretty the same.

[mholt]

@MisterDuval

I understand, but as this %00 sign is not allowed in URLs, as RFC tells, we should better block it before passing it to backends

That's not what the RFC says, though. It says (emphasis mine):

the "%00" percent-encoding (NUL) may require special handling and should be rejected if the application is not expecting to receive raw data within a component.

But the reverse proxy is not the application. We don't know what the application expects. And when we've meddled with the URL before, it has caused problems.

My problem is that we cannot even block the %00 request with a Caddy configuration, if not fixed in core, as this character is not matchable (from my tests, maybe you could try to?).

Are you sure about this? What did you try? I'll see if I can reproduce it.

I'm going to close this PR since I think a fair amount of discussion / research / further information needs to be ascertained first, before we act on it (if at all). But feel free to continue the discussion.

[MisterDuval]

@mholt @francislavoie what do we do on that one? I have hundreds of setups of Caddy/Windows/PHP with a huge flaw. We need to fix that. I don't understand that position you're standing?

[francislavoie]

Like we said, we need a better understanding of why it's a problem. If we don't actually understand the problem, then we can't be sure that any fix is actually safe. We don't have the tooling to find that out, so honestly, the onus is on you to help us figure out deeper specifics.

Any change we make here to the Caddy project will need to be a change that us, the maintainers of the project, will need to live with and continue to ensure works correctly alongside any other changes made to nearby code. That's a burden we don't want when we don't actually understand why we need that code change.

[MisterDuval]

I have a new showcase, much simplier, with a flaw Nginx had in the past (2010):
https://nealpoole.com/blog/2011/08/possible-arbitrary-code-execution-with-null-bytes-php-and-old-versions-of-nginx/

So the flaw is on Caddy Windows + PHP (whatever version) :
http://127.0.0.1/win.jpg%00.php (null character)
win.jpg contains
<?php echo 'hello flaw';
Caddy outputs the script execution:
hello flaw

Please consider this flaw as critical @mholt

You can avoid this flaw on your setup by adding a index.php to your root www folder. (don't know why btw)

[mholt]

I'm not able to access Windows for at least 4 days or so due to family circumstances.

@MisterDuval Does the flaw only reproduce on Windows 10 Pro or Home? I need to check which one I'll be using but I won't be able to have access to both. Independent verification would be ideal here.

[francislavoie]

Okay, I can replicate it with that, Win10 Home, PHP 8.0.

I'm still not certain what the right fix is here. But I think the least risky for now would be to trigger a 400 Bad Request in the fastcgi module if a null byte is found in the path.

I opened #4614 with that proposed fix for now.