fix: Address security audit findings (1.2) - #371
Draft
nielsenko wants to merge 16 commits into
Draft
Conversation
Contributor
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
use() on a catch-all was skipped for the prefix itself, so /api bypassed the middleware that /api/keys ran.
Backported from serverpod#360 (RFC compliant typed headers) so the header-parsing security fixes can build on the same primitives as main. The files are deliberately not exported from relic_core.dart: they are an implementation detail here, and keeping them internal leaves the 1.2.x public API surface unchanged.
The unanchored regexes backtracked quadratically, wedging the isolate on a long value. Text between auth-params is now rejected, not skipped.
Sections were materialised before the response returned, and the count was unbounded, so a short request could ask for many times the file size.
Random().nextInt(1000000) gave ~20 bits, so boundaries repeated within a few thousand responses.
Returning the future from inside the try skipped the catch, so a failed response write left the connection open and the client waiting.
Rejects CR, LF and NUL where every header write funnels, so the defence is Relic own rather than inherited from dart:io.
No such check exists. The page now says every file in the directory is served, including dot files.
The host was only segment 0 of a flat string, so a path resolving upwards could pop it and select another virtual host.
Decoding first turned %2F into a separator no upstream proxy ever saw. NormalizedPath.fromUri names the safe conversion.
Refuse a mismatched Origin with 403; opt out with allowAnyOrigin. Only the host is compared, since a TLS-terminating proxy changes scheme and port. Breaking for apps relying on cross-origin sockets.
Emitted through ParameterValue, so a quote in a filename can no longer end the value and append a filename*. Parsing is now quote-aware too, or Relic could not read its own output.
An unescaped realm could close its value and graft a second challenge onto the response.
io.ContentType takes the parts separately, so they bypassed the CR/LF check dart:io applies to ordinary header values.
Hijacking and upgrading detach the connection from the underlying server, which then reports and closes nothing, so a hijacked socket survived close(force: true) and connectionsInfo() could not show it. Track them in the adapter: a forced close destroys them, a graceful close drains, sends 1001 to WebSocket peers, then closes the rest after a bounded wait. 40 hijack cycles ignoring the socket leaked 80 sockets before, none after.
The teardown closed with a 5s timeout and, on timeout, retried close with none, so a shutdown that could not complete hung the suite instead of reporting it. Close forcefully once: this test deliberately leaves the hijacked connection open. The timeout expectation was also never awaited, so it raced the teardown.
nielsenko
force-pushed
the
fix-sec-findings-1.2
branch
from
August 10, 2026 13:53
e9ab5d1 to
095f221
Compare
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.