mssql: prevent uint16 overflow in PRELOGIN option bounds check - #758
Open
ChrisJr404 wants to merge 1 commit into
Open
mssql: prevent uint16 overflow in PRELOGIN option bounds check#758ChrisJr404 wants to merge 1 commit into
ChrisJr404 wants to merge 1 commit into
Conversation
Signed-off-by: Chris (ChrisJr404) <11917633+ChrisJr404@users.noreply.github.com>
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 malformed MSSQL PRELOGIN response can crash a scan because the offset+length bounds check in
decodePreloginOptionsis computed inuint16arithmetic and can overflow, so this widens both values tointbefore the check.offsetandlengthare read straight from the server's PRELOGIN response asuint16. The guardif len(body) < int(offset+length)evaluatesoffset+lengthas auint16, so a large offset wraps around instead of failing the check. For exampleoffset=0xFFFF, length=0x0002wraps to1, the check passes, and the next linebody[offset : offset+length]slicesbody[65535:1], panicking withslice bounds out of range [65535:1]. Since a scanned server fully controls these bytes, any host can trigger the panic. The fix convertsoffsetandlengthtointat read time so the addition can't wrap, and the existing check then correctly rejects the packet withErrInvalidData.How to Test
go test ./modules/mssql/Two new tests in
modules/mssql/connection_test.gocover it:TestDecodePreloginOptionsOverflowfeeds the overflowing option above and assertsErrInvalidDatais returned (before the fix this test panics).TestDecodePreloginOptionsValidconfirms a well-formed PRELOGIN body still decodes to the right option value.Notes & Caveats
Behavior is unchanged for valid packets; only inputs that previously panicked now return
ErrInvalidData, matching how other malformed PRELOGIN bodies are already handled in this function.Issue Tracking
No existing issue; found while reviewing the PRELOGIN parser.