Repository navigation
pg: a backend message longer than MaxReceiveBytes fails the connection, and its length can no longer wrap - #315
Merged
Merged
Conversation
…n, and its length can no longer wrap TryReadMessage checked for a complete message with position + 1 + length > buffer.Length. A length near int.MaxValue wraps that sum negative, so a 9-byte message announcing 2 GiB was taken as fully arrived, with a body span of 2 GiB over the 64 KiB receive buffer and _scan set to int.MinValue. During startup the next parse threw IndexOutOfRangeException. A DataRow framed that way during a query was worse: ReadFirstField decoded its first field straight out of that span, and the process died with a fatal AccessViolationException. A declared length above MaxReceiveBytes, the documented ceiling on one backend message, now fails the connection with a PgException that names it, and the completeness check is a subtraction that cannot overflow. Negative lengths were already refused as malformed. No new state. New tests in PgFramingTests run against a FakeServer, a new harness helper for scripted loopback peers: a startup answered with an AuthenticationOk whose length is int.MaxValue, and a control in which the same server's well-formed startup connects. Confirmed both ways: on f298af0 the test gets IndexOutOfRangeException, and with the fix a PgException that names the oversized message. The DataRow case was checked with a probe that is not committed: on f298af0 the test process aborted, and with the fix the query fails with the same PgException.
# Conflicts: # tests/Ioxide.Tests.Pg/Program.cs
Merged
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
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.
A pg server that sent a message with a length near
int.MaxValuecould crash the whole process: one 13-byte DataRow was enough.TryReadMessagechecked for a complete message withposition + 1 + length > buffer.Length. A length nearint.MaxValuewraps that sum negative, so a message announcing 2 GiB was taken as already arrived. Its body became a 2 GiB span over the 64 KiB receive buffer, and_scanwas set toint.MinValue. During startup, the next parse then threwIndexOutOfRangeException. During a query,ReadFirstFielddecoded the DataRow's first field straight out of that span. Reading past the buffer killed the process with a fatalAccessViolationException, which no handler can catch.A declared length above
MaxReceiveBytes, which the docs already call the hard ceiling on one backend message, now fails the connection with aPgExceptionthat names the message. The completeness check is now a subtraction that cannot overflow. Negative lengths were already refused as malformed, and still are. No new state.Tests
New, in
PgFramingTests(no postgres needed):pg: a backend message whose length overflows the frame fails the connection with a clear errorcontrol: the same fake server's well-formed startup connectsBoth run against
FakeServer, a new harness helper: a loopback listener that runs a script per accepted connection on its own thread. Here the script answers the startup message with an AuthenticationOk whose length isint.MaxValue. The control's script answers with a well-formed AuthenticationOk and ReadyForQuery, so the refusal is about the length and not the fake server.int.MaxValueIndexOutOfRangeException: Index was outside the bounds of the array.PgException: backend message 'R' of 2147483647 bytes exceeds 67108864 bytesFatal error. System.AccessViolationExceptioninReadFirstFieldPgException: backend message 'D' of 2147483647 bytes exceeds 67108864 bytesSuites: Unit 63 (1 pending, as on main), Pg 8, Redis 3, with both sidecars running (nothing skipped).
Hot path, barely: one extra integer compare per backend message.
FakeServer.csis byte-identical in the other hostile-server branches (SCRAM iterations, RESP depth, open deadline), so merging them adds the file once.