diff options
author | Jeff King <peff@peff.net> | 2020-03-27 08:03:38 (GMT) |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2020-03-27 19:18:48 (GMT) |
commit | 4845b7724582a315eb4eb13d5058f85d21798e94 (patch) | |
tree | 8d27c4687e289d16d784aa1bf4c86be160ab532d /t/t5704-protocol-violations.sh | |
parent | 88124ab263670b4252be7c13d03754a127cee90e (diff) | |
download | git-4845b7724582a315eb4eb13d5058f85d21798e94.zip git-4845b7724582a315eb4eb13d5058f85d21798e94.tar.gz git-4845b7724582a315eb4eb13d5058f85d21798e94.tar.bz2 |
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>
Diffstat (limited to 't/t5704-protocol-violations.sh')
-rwxr-xr-x | t/t5704-protocol-violations.sh | 33 |
1 files changed, 33 insertions, 0 deletions
diff --git a/t/t5704-protocol-violations.sh b/t/t5704-protocol-violations.sh new file mode 100755 index 0000000..950cfb2 --- /dev/null +++ b/t/t5704-protocol-violations.sh @@ -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 |