Repository navigation
Fixed stack buffer overflow in GetServerLiteral for TLS VAR requests - #6423
Open
Dhanu-dynamic wants to merge 1 commit into
Open
Dhanu-dynamic wants to merge 1 commit into
Dhanu-dynamic wants to merge 1 commit into
Conversation
GetServerLiteral() zeroed and formatted CF_BUFSIZE bytes into the sendbuffer it was handed, but BusyWithNewProtocol() allocates that buffer as CF_BUFSIZE - CF_INBAND_OFFSET bytes (the most a single transaction can carry). Any VAR request admitted by the literals ACL therefore wrote 8 bytes past the end of the handler's stack buffer. Pass the buffer size into GetServerLiteral(), as DoExec2() already does, and bound the writes to it. Both callers pass sizeof(sendbuffer). Changelog: Title Signed-off-by: Dhanraj E <dhanraj@labs.digiscrypt.com>
|
Thanks for submitting a pull request! Maybe @craigcomstock can review this? |
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
2 participants
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.
the VAR handler in BusyWithNewProtocol (server_tls.c) declares its sendbuffer as CF_BUFSIZE - CF_INBAND_OFFSET bytes, the most one transaction can carry, but GetServerLiteral in server_common.c zeroes and snprintf()s CF_BUFSIZE bytes into whatever buffer it is given. so any VAR request admitted by the literals ACL writes 8 bytes past the end of that stack buffer in the root cf-serverd; in the community build the ReturnLiteralData stub makes the "BAD: Not found" branch do it on every such request, and in Enterprise a literal of 4088 bytes or more overflows through the snprintf too. the classic caller in BusyWithClassicConnection hands over a full CF_BUFSIZE buffer and is unaffected. pass the buffer size into GetServerLiteral the same way DoExec2 already takes sendbuf_size and bound the writes to it; both callers pass sizeof(sendbuffer). nothing is lost, since SendTransaction refuses anything longer than CF_MSGSIZE anyway. the regression test in protocol_test.c runs GetServerLiteral over a socketpair with a CF_MSGSIZE buffer followed by guard bytes: before the change the guard bytes read 0 instead of 0x5a, after it they are untouched and the reply is still "BAD: Not found". full unit suite passes under AddressSanitizer with -Werror.