laminas-diactoros
laminas-diactoros copied to clipboard
laminas
Allow double slashes in the path according to the RFC3986
| Q | A |
|---|---|
| Documentation | no |
| Bugfix | yes |
| BC Break | maybe |
| New Feature | no |
| RFC | no |
| QA | no |
Description
This resolves #74
maybe it could be a BC break, because the behavior of the getPath has changed.
its since this commit: https://github.com/laminas/laminas-diactoros/commit/68fc7424a66735926589dbaf74c0659c09e75764
I've added this to the 2.9.0 milestone to prevent an accidental release that sails out without this, waiting for feedback first.
@laminas/technical-steering-committee I don't know what to do with this one: can I get an opinion, please?
I'd consider the PSR-7 test suite in error here, as it leads to an XSS vulnerability, and I recommend providing them our security test instead.
The part that particularly concerns me is the removal of the security related test - this would be not just a BC break, but reintroduction of a previously patched vulnerability.
I'd consider the PSR-7 test suite in error here, as it leads to an XSS vulnerability, and I recommend providing them our security test instead.
I think the tricky thing here is context since the XSS vulnerability happens depending on the usage.
A URI that looks like https://example.org//google.com is completely fine according to the RFC but might introduce the vulnerability if only the path is used to redirect users.
My question, though, is: should a low-level component such as a PSR-7 implementation be responsible for preventing those redirection vulnerabilities? If so, we should probably look at solving this on the specification level IMHO.
should a low-level component such as a PSR-7 implementation be responsible for preventing those redirection vulnerabilities? If so, we should probably look at solving this on the specification level IMHO.
I think it should, as:
- A URI instance can be used to make client requests; if the HTTP client is not robust, we've created a vulnerability for them.
- The string representation is often used to create URIs for API payloads and/or web pages; again, if we spit these out without normalization, we create a vulnerability for them.
This latter is exactly what was reported, and the CVE was not limited to Diactoros, but was also reported against a number of libraries that generate/represent URIs.
In terms of solving it at the specification level, that's absolutely do-able (we recently did it for a class of vulnerabilities related to multi-line headers); I think we can still push a patch now to the upstream PSR-7 test suite with the test we created when patching the vulnerability reported against us.
Maybe handled by #128 ? Unsure
Yes - #128 provides the correct solution for this.