Skip to content

Commit

Permalink
Patch Rewards Program submission for rs/cors#171
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 705145419
  • Loading branch information
Googler authored and copybara-github committed Dec 11, 2024
1 parent 0f469c8 commit 4faa88d
Showing 1 changed file with 109 additions and 0 deletions.
109 changes: 109 additions & 0 deletions patch-rewards-program/rewarded-patches/rs/cors/336848281.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@

---

### 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

0 comments on commit 4faa88d

Please sign in to comment.