-
Notifications
You must be signed in to change notification settings - Fork 6
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Patch Rewards Program submission for rs/cors#171
PiperOrigin-RevId: 704444981
- Loading branch information
1 parent
0c64b61
commit 4b994ec
Showing
1 changed file
with
108 additions
and
0 deletions.
There are no files selected for viewing
108 changes: 108 additions & 0 deletions
108
patch-rewards-program/rewarded-patches/rs/cors/336848281.md
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,108 @@ | ||
--- | ||
|
||
### Improvement category | ||
|
||
#### Submission title | ||
|
||
Patch rewards submission: patching rs/cors (DoS via malicious preflight requests) | ||
|
||
|
||
#### Please select the best-matching category: | ||
|
||
Other | ||
|
||
|
||
--- | ||
|
||
### Patch details | ||
|
||
|
||
#### Specify project (the scope is limited to the listed projects): | ||
|
||
The reference implementation of Certificate Transparency and its open source dependencies | ||
|
||
|
||
#### Project name | ||
|
||
github.com/rs/cors | ||
|
||
|
||
#### Patch name or short description | ||
|
||
Normalize allowed request headers and store them in a sorted set (fixes #170) | ||
|
||
|
||
#### Links to the diffs of the patch | ||
|
||
https://github.com/rs/cors/commit/4c32059b2756926619f6bf70281b91be7b5dddb2 | ||
|
||
|
||
#### Complexity – select the option that best describes the patch's complexity: | ||
|
||
Modest effort or complexity | ||
|
||
|
||
#### Impact – select the option that best describes the security impact of the patch: | ||
|
||
Compelling security benefits | ||
|
||
|
||
#### Tell us more about the patch | ||
|
||
The [Go reference implementation of Certificate Transparency](https://github.com/google/certificate-transparency-go) counts [github.com/rs/cors](github.com/rs/cors), a CORS middleware library, as one of its open-source dependencies. The project [recently updated](https://github.com/google/certificate-transparency-go/commit/9b5b39e97764db78eca1ffe0141ab177dbe849fa) from rs/cors v1.10.1 (vulnerable) to rs/cors v1.11.0 (no longer vulnerable thanks to my patch). | ||
|
||
My patch consists in processing the `Access-Control-Request-Headers` header of CORS-preflight requests in a defensive manner. More specifically, the patch takes advantage of [guarantees from the Fetch standard](https://fetch.spec.whatwg.org/#cors-unsafe-request-header-names) about legitimate CORS-preflight requests: | ||
- CORS-preflight requests contain at most one `Access-Control-Request-Headers` header; | ||
- The value of that header consists in unique byte-lowercase comma-separated tokens sorted in lexicographical order. | ||
|
||
Before my patch was merged, rs/cors middleware would incur excessive heap allocations when processing spoofed preflight requests that include a maliciously long `Access-Control-Request-Headers` header value (but still within [`http.DefaultMaxHeaderBytes`](https://pkg.go.dev/net/http.DefaultMaxHeaderBytes)). Such behaviour could be abused by adversaries to make the server run out of memory and crash. Note that, since CORS middleware must sit in front of any authentication for CORS preflight to succeed (because [CORS-preflight request don't carry credentials](https://fetch.spec.whatwg.org/#cors-protocol-and-credentials)), attackers wouldn't even need to be authenticated. | ||
|
||
Here are some benchmarks illustrating the effects of my patch; note the dramatic reduction in heap allocations on the line labeled _PreflightAdversarialACRH-8_ : | ||
|
||
```text | ||
goos: darwin | ||
goarch: amd64 | ||
pkg: github.com/rs/cors | ||
cpu: Intel(R) Core(TM) i7-6700HQ CPU @ 2.60GHz | ||
│ before │ after │ | ||
│ sec/op │ sec/op vs base │ | ||
Without-8 5.290n ± 15% 4.528n ± 31% ~ (p=0.183 n=10) | ||
Default-8 92.34n ± 3% 95.86n ± 3% +3.81% (p=0.043 n=10) | ||
AllowedOrigin-8 126.8n ± 5% 128.6n ± 3% ~ (p=0.541 n=10) | ||
Preflight-8 308.3n ± 1% 295.8n ± 1% -4.07% (p=0.000 n=10) | ||
PreflightHeader-8 366.5n ± 1% 333.4n ± 5% -9.02% (p=0.000 n=10) | ||
PreflightAdversarialACRH-8 33214.5n ± 0% 360.9n ± 4% -98.91% (p=0.000 n=10) | ||
Wildcard/match-8 9.723n ± 16% 9.919n ± 12% ~ (p=0.225 n=10) | ||
Wildcard/too_short-8 0.9875n ± 0% 0.9923n ± 0% +0.48% (p=0.034 n=10) | ||
geomean 82.91n 45.86n -44.69% | ||
│ before │ after │ | ||
│ B/op │ B/op vs base │ | ||
Without-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ | ||
Default-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ | ||
AllowedOrigin-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ | ||
Preflight-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ | ||
PreflightHeader-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ | ||
PreflightAdversarialACRH-8 36904.00 ± 0% 32.00 ± 0% -99.91% (p=0.000 n=10) | ||
Wildcard/match-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ | ||
Wildcard/too_short-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ | ||
geomean ² -58.58% ² | ||
¹ all samples are equal | ||
² summaries must be >0 to compute geomean | ||
``` | ||
|
||
More details: | ||
- https://github.com/rs/cors/issues/170 | ||
- https://github.com/rs/cors/pull/171 | ||
|
||
--- | ||
|
||
### Credit | ||
|
||
jub0bs.com | ||
|
||
--- | ||
|
||
### Reward Amount | ||
|
||
$5000.00 -- moderately complex patch that offer compelling security benefits |