-
Notifications
You must be signed in to change notification settings - Fork 25.3k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
upload-pack: handle unexpected delim packets
When processing the arguments list for a v2 ls-refs or fetch command, we loop like this: while (packet_reader_read(request) != PACKET_READ_FLUSH) { const char *arg = request->line; ...handle arg... } to read and handle packets until we see a flush. The hidden assumption here is that anything except PACKET_READ_FLUSH will give us valid packet data to read. But that's not true; PACKET_READ_DELIM or PACKET_READ_EOF will leave packet->line as NULL, and we'll segfault trying to look at it. Instead, we should follow the more careful model demonstrated on the client side (e.g., in process_capabilities_v2): keep looping as long as we get normal packets, and then make sure that we broke out of the loop due to a real flush. That fixes the segfault and correctly diagnoses any unexpected input from the client. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
- Loading branch information
Showing
3 changed files
with
41 additions
and
2 deletions.
There are no files selected for viewing
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
#!/bin/sh | ||
|
||
test_description='Test responses to violations of the network protocol. In most | ||
of these cases it will generally be acceptable for one side to break off | ||
communications if the other side says something unexpected. We are mostly | ||
making sure that we do not segfault or otherwise behave badly.' | ||
. ./test-lib.sh | ||
|
||
test_expect_success 'extra delim packet in v2 ls-refs args' ' | ||
{ | ||
packetize command=ls-refs && | ||
printf 0001 && | ||
# protocol expects 0000 flush here | ||
printf 0001 | ||
} >input && | ||
test_must_fail env GIT_PROTOCOL=version=2 \ | ||
git upload-pack . <input 2>err && | ||
test_i18ngrep "expected flush after ls-refs arguments" err | ||
' | ||
|
||
test_expect_success 'extra delim packet in v2 fetch args' ' | ||
{ | ||
packetize command=fetch && | ||
printf 0001 && | ||
# protocol expects 0000 flush here | ||
printf 0001 | ||
} >input && | ||
test_must_fail env GIT_PROTOCOL=version=2 \ | ||
git upload-pack . <input 2>err && | ||
test_i18ngrep "expected flush after fetch arguments" err | ||
' | ||
|
||
test_done |
This file contains 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