When we read acks from the remote, we expect either:
ACK <sha1>
or
ACK <sha1> <multi-ack-flag>
We parse the "ACK <sha1>" bit from the line, and then start
looking for the flag strings at "line+45"; if we don't have
them, we assume it's of the first type. But if we do have
the first type, then line+45 is not necessarily inside our
string at all!
It turns out that this works most of the time due to the way
we parse the packets. They should come in with a newline,
and packet_read puts an extra NUL into the buffer, so we end
up with:
ACK <sha1>\n\0
with the newline at offset 44 and the NUL at offset 45. We
then strip the newline, putting a NUL at offset 44. So
when we look at "line+45", we are looking past the end of
our string; but it's OK, because we hit the terminator from
the original string.
This breaks down, however, if the other side does not
terminate their packets with a newline. In that case, our
packet is one character shorter, and we start looking
through uninitialized memory for the flag. No known
implementation sends such a packet, so it has never come up
in practice.
This patch tightens the check by looking for a short,
flagless ACK before trying to parse the flag.
Signed-off-by: Jeff King <peff@xxxxxxxx>
---
This is the absolute minimal fix, which just checks for the no-flag case
early; we still treat arbitrary crud in the flag field as just an ACK.
>From my understanding of the protocol, a saner parsing scheme would be:
const char *flag = line + 44; /* we already parsed "ACK <sha1>" */
if (!*flag)
return ACK;
if (!strcmp(flag, " continue"))
return ACK_continue;
if (!strcmp(flag, " common"))
return ACK_continue;
if (!strcmp(flag, " ready"))
return ACK_ready;
die("fetch-pack expected multi-ack flag, got: %s", line);
But that is much tighter, and I wasn't sure if the looseness was there
to facilitate future expansion or something (though I'd think we would
need a new capability for that).
fetch-pack.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/fetch-pack.c b/fetch-pack.c
index 6d8926a..27a3e80 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -226,6 +226,8 @@ static enum ack_type get_ack(int fd, unsigned char *result_sha1)
return NAK;
if (!prefixcmp(line, "ACK ")) {
if (!get_sha1_hex(line+4, result_sha1)) {
+ if (len < 45)
+ return ACK;
if (strstr(line+45, "continue"))
return ACK_continue;
if (strstr(line+45, "common"))
--
1.8.2.rc0.9.g352092c
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html