From b0df0c16ead4c5512d506dcbbdf31194d992803c Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Tue, 19 May 2020 06:54:00 -0400 Subject: stateless-connect: send response end packet Currently, remote-curl acts as a proxy and blindly forwards packets between an HTTP server and fetch-pack. In the case of a stateless RPC connection where the connection is terminated before the transaction is complete, remote-curl will blindly forward the packets before waiting on more input from fetch-pack. Meanwhile, fetch-pack will read the transaction and continue reading, expecting more input to continue the transaction. This results in a deadlock between the two processes. This can be seen in the following command which does not terminate: $ git -c protocol.version=2 clone https://github.com/git/git.git --shallow-since=20151012 Cloning into 'git'... whereas the v1 version does terminate as expected: $ git -c protocol.version=1 clone https://github.com/git/git.git --shallow-since=20151012 Cloning into 'git'... fatal: the remote end hung up unexpectedly Instead of blindly forwarding packets, make remote-curl insert a response end packet after proxying the responses from the remote server when using stateless_connect(). On the RPC client side, ensure that each response ends as described. A separate control packet is chosen because we need to be able to differentiate between what the remote server sends and remote-curl's control packets. By ensuring in the remote-curl code that a server cannot send response end packets, we prevent a malicious server from being able to perform a denial of service attack in which they spoof a response end packet and cause the described deadlock to happen. Reported-by: Force Charlie Helped-by: Jeff King Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- connect.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) (limited to 'connect.c') diff --git a/connect.c b/connect.c index 11c6ec70a0..0df45a1108 100644 --- a/connect.c +++ b/connect.c @@ -406,10 +406,21 @@ out: return ret; } +void check_stateless_delimiter(int stateless_rpc, + struct packet_reader *reader, + const char *error) +{ + if (!stateless_rpc) + return; /* not in stateless mode, no delimiter expected */ + if (packet_reader_read(reader) != PACKET_READ_RESPONSE_END) + die("%s", error); +} + struct ref **get_remote_refs(int fd_out, struct packet_reader *reader, struct ref **list, int for_push, const struct argv_array *ref_prefixes, - const struct string_list *server_options) + const struct string_list *server_options, + int stateless_rpc) { int i; *list = NULL; @@ -446,6 +457,9 @@ struct ref **get_remote_refs(int fd_out, struct packet_reader *reader, if (reader->status != PACKET_READ_FLUSH) die(_("expected flush after ref listing")); + check_stateless_delimiter(stateless_rpc, reader, + _("expected response end packet after ref listing")); + return list; } -- cgit v1.2.3