Commit ee11d47e by Patrick Steinhardt

tag: fix out of bounds read when searching for tag message

When parsing tags, we skip all unknown fields that appear before the tag
message. This skipping is done by using a plain `strstr(buffer, "\n\n")`
to search for the two newlines that separate tag fields from tag
message. As it is not possible to supply a buffer length to `strstr`,
this call may skip over the buffer's end and thus result in an out of
bounds read. As `strstr` may return a pointer that is out of bounds, the
following computation of `buffer_end - buffer` will overflow and result
in an allocation of an invalid length.

Fix the issue by using `git__memmem` instead. Add a test that verifies
parsing the tag fails not due to the allocation failure but due to the
tag having no message.
parent 4c738e56
...@@ -70,10 +70,9 @@ static int tag_parse(git_tag *tag, const char *buffer, const char *buffer_end) ...@@ -70,10 +70,9 @@ static int tag_parse(git_tag *tag, const char *buffer, const char *buffer_end)
static const char *tag_types[] = { static const char *tag_types[] = {
NULL, "commit\n", "tree\n", "blob\n", "tag\n" NULL, "commit\n", "tree\n", "blob\n", "tag\n"
}; };
unsigned int i;
size_t text_len, alloc_len; size_t text_len, alloc_len;
char *search; const char *search;
unsigned int i;
if (git_oid__parse(&tag->target, &buffer, buffer_end, "object ") < 0) if (git_oid__parse(&tag->target, &buffer, buffer_end, "object ") < 0)
return tag_error("object field invalid"); return tag_error("object field invalid");
...@@ -138,8 +137,9 @@ static int tag_parse(git_tag *tag, const char *buffer, const char *buffer_end) ...@@ -138,8 +137,9 @@ static int tag_parse(git_tag *tag, const char *buffer, const char *buffer_end)
tag->message = NULL; tag->message = NULL;
if (buffer < buffer_end) { if (buffer < buffer_end) {
/* If we're not at the end of the header, search for it */ /* If we're not at the end of the header, search for it */
if( *buffer != '\n' ) { if(*buffer != '\n') {
search = strstr(buffer, "\n\n"); search = git__memmem(buffer, buffer_end - buffer,
"\n\n", 2);
if (search) if (search)
buffer = search + 1; buffer = search + 1;
else else
......
...@@ -198,3 +198,21 @@ void test_object_tag_parse__missing_message_fails(void) ...@@ -198,3 +198,21 @@ void test_object_tag_parse__missing_message_fails(void)
"tagger taggy@taggart.com>\n"; "tagger taggy@taggart.com>\n";
assert_tag_fails(tag, 0); assert_tag_fails(tag, 0);
} }
void test_object_tag_parse__no_oob_read_when_searching_message(void)
{
const char *tag =
"object a8d447f68076d1520f69649bb52629941be7031f\n"
"type tag\n"
"tag \n"
"tagger <>\n"
" \n\n"
"Message";
/*
* The OOB read previously resulted in an OOM error. We
* thus want to make sure that the resulting error is the
* expected one.
*/
assert_tag_fails(tag, strlen(tag) - strlen("\n\nMessage"));
cl_assert(strstr(giterr_last()->message, "tag contains no message"));
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment