From 7d02019a07840c0263ab935f626af8002af12a91 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 6 Jun 2016 12:59:17 +0200 Subject: [PATCH 1/2] transports: smart: fix potential invalid memory dereferences When we receive a packet of exactly four bytes encoding its length as those four bytes it can be treated as an empty line. While it is not really specified how those empty lines should be treated, we currently ignore them and do not return an error when trying to parse it but simply advance the data pointer. Callers invoking `git_pkt_parse_line` are currently not prepared to handle this case as they do not explicitly check this case. While they could always reset the passed out-pointer to `NULL` before calling `git_pkt_parse_line` and determine if the pointer has been set afterwards, it makes more sense to update `git_pkt_parse_line` to set the out-pointer to `NULL` itself when it encounters such an empty packet. Like this it is guaranteed that there will be no invalid memory references to free'd pointers. As such, the issue has been fixed such that `git_pkt_parse_line` always sets the packet out pointer to `NULL` when an empty packet has been received and callers check for this condition, skipping such packets. --- src/transports/smart_pkt.c | 1 + src/transports/smart_protocol.c | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/src/transports/smart_pkt.c b/src/transports/smart_pkt.c index 2ea57bb64..2297cc94f 100644 --- a/src/transports/smart_pkt.c +++ b/src/transports/smart_pkt.c @@ -433,6 +433,7 @@ int git_pkt_parse_line( * line? */ if (len == PKT_LEN_SIZE) { + *head = NULL; *out = line; return 0; } diff --git a/src/transports/smart_protocol.c b/src/transports/smart_protocol.c index 02e1ecf74..3448fa7fb 100644 --- a/src/transports/smart_protocol.c +++ b/src/transports/smart_protocol.c @@ -759,6 +759,14 @@ static int add_push_report_sideband_pkt(git_push *push, git_pkt_data *data_pkt, line_len -= (line_end - line); line = line_end; + /* When a valid packet with no content has been + * read, git_pkt_parse_line does not report an + * error, but the pkt pointer has not been set. + * Handle this by skipping over empty packets. + */ + if (pkt == NULL) + continue; + error = add_push_report_pkt(push, pkt); git_pkt_free(pkt); @@ -813,6 +821,9 @@ static int parse_report(transport_smart *transport, git_push *push) error = 0; + if (pkt == NULL) + continue; + switch (pkt->type) { case GIT_PKT_DATA: /* This is a sideband packet which contains other packets */ From 13deb8745d6b604a7fc45bb7ddee2a2052e80000 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 7 Jun 2016 08:35:26 +0200 Subject: [PATCH 2/2] index: fix NULL pointer access in index_remove_entry When removing an entry from the index by its position, we first retrieve the position from the index's entries and then try to remove the retrieved value from the index map with `DELETE_IN_MAP`. When `index_remove_entry` returns `NULL` we try to feed it into the `DELETE_IN_MAP` macro, which will unconditionally call `idxentry_hash` and then happily dereference the `NULL` entry pointer. Fix the issue by not passing a `NULL` entry into `DELETE_IN_MAP`. --- src/index.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/index.c b/src/index.c index 20ab6a19d..32f585faf 100644 --- a/src/index.c +++ b/src/index.c @@ -505,10 +505,11 @@ static int index_remove_entry(git_index *index, size_t pos) int error = 0; git_index_entry *entry = git_vector_get(&index->entries, pos); - if (entry != NULL) + if (entry != NULL) { git_tree_cache_invalidate_path(index->tree, entry->path); + DELETE_IN_MAP(index, entry); + } - DELETE_IN_MAP(index, entry); error = git_vector_remove(&index->entries, pos); if (!error) {