fastcgi: Protect against requests with null bytes in the path

[francislavoie]

Followup to #4574, credit to @MisterDuval for reporting the issue and finding a reproduce case.

[mholt]

Thank you Francis. It would be much appreciated if @MisterDuval could confirm that this patch fixes the issue for him, too.

[MisterDuval]

@mholt that's OK for me, I'm getting a "400 Bad Request", thanks for this patch @francislavoie !

[midnight-wonderer]

FWIW, this class of issue is called parser differential; it is considered to be a security flaw.
You may fix it now for FastCGI by making the parser inclined to what FastCGI expects, but as long as the differential exists, there might be a way to exploit it; better inform security personnel if you don't do it already.
The proper fix might be better implemented at the upstream server.

[francislavoie]

I completely agree @midnight-wonderer, as we've said in the linked issue. I really believe this should be fixed in PHP's fastcgi implementation.

[mholt]

Yep, also a hard agree here.

[MisterDuval]

If we can avoid a flaw, why wait for different fastcgi backends... Nginx does the same as Caddy now, even if the real flaw is after Caddy, we can avoid it here. Le dim. 20 mars 2022, 05:11, Matt Holt ***@***.***> a écrit :

…

Yep, also a hard agree here. — Reply to this email directly, view it on GitHub <#4614 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAQKYRGTDM2SI5KBJBDU4LVA2QP7ANCNFSM5P7A7OGA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[midnight-wonderer]

Because by avoiding it here, you may introduce other vulnerabilities to everyone else.
The case of FastCGI in the example is only one specific case, where you have FastCGI on the inside with Caddy as a frontend server.

In a not so simple case, someone may use Caddy internally between their own services; in that setup, making Caddy reject the request may have other implications. For example, it might mean an attacker can make their application break midway; did something but exit abruptly, which might imply vulnerabilities depending on the context.

The nature of parser differential is the differential part; by propagating the difference from FastCGI to Caddy, the issue might simply move around. Worse, escalating, the differential will be used in bigger vulnerability chains.

As long as the differences exist, people will find a way to exploit them.
Here is an example of how it has been exploits in a browser: https://youtu.be/lG7U3fuNw3A
Who would have thought, right? This is even stemmed from the HTML specs themselves.