Skip to content

Commit fa7aba7

Browse files
pks-tpks-t
authored
Merge pull request libgit2#4871 from pks-t/pks/tree-parsing-fixes
Tree parsing fixes
2 parents b5ae83b + 7fafec0 commit fa7aba7

File tree

4 files changed

+239
-26
lines changed

4 files changed

+239
-26
lines changed

‎src/tree.c‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -356,21 +356,21 @@ static int tree_error(const char *str, const char *path)
356356
return -1;
357357
}
358358

359-
static int parse_mode(unsigned int *modep, const char *buffer, const char **buffer_out)
359+
static int parse_mode(uint16_t *mode_out, const char *buffer, size_t buffer_len, const char **buffer_out)
360360
{
361-
unsigned char c;
362-
unsigned int mode = 0;
361+
int32_t mode;
362+
int error;
363363

364-
if (*buffer == ' ')
364+
if (!buffer_len || git__isspace(*buffer))
365365
return -1;
366366

367-
while ((c = *buffer++) != ' ') {
368-
if (c < '0' || c > '7')
369-
return -1;
370-
mode = (mode << 3) + (c - '0');
371-
}
372-
*modep = mode;
373-
*buffer_out = buffer;
367+
if ((error = git__strntol32(&mode, buffer, buffer_len, buffer_out, 8)) < 0)
368+
return error;
369+
370+
if (mode < 0 || mode > UINT16_MAX)
371+
return -1;
372+
373+
*mode_out = mode;
374374

375375
return 0;
376376
}
@@ -392,11 +392,14 @@ int git_tree__parse_raw(void *_tree, const char *data, size_t size)
392392
git_tree_entry *entry;
393393
size_t filename_len;
394394
const char *nul;
395-
unsigned int attr;
395+
uint16_t attr;
396396

397-
if (parse_mode(&attr, buffer, &buffer) < 0 || !buffer)
397+
if (parse_mode(&attr, buffer, buffer_end - buffer, &buffer) < 0 || !buffer)
398398
return tree_error("failed to parse tree: can't parse filemode", NULL);
399399

400+
if (buffer >= buffer_end || (*buffer++) != ' ')
401+
return tree_error("failed to parse tree: missing space after filemode", NULL);
402+
400403
if ((nul = memchr(buffer, 0, buffer_end - buffer)) == NULL)
401404
return tree_error("failed to parse tree: object is corrupted", NULL);
402405

‎src/util.c‎

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,11 @@ int git__strntol64(int64_t *result, const char *nptr, size_t nptr_len, const cha
8383
/*
8484
* White space
8585
*/
86-
while (git__isspace(*p))
87-
p++;
86+
while (nptr_len && git__isspace(*p))
87+
p++, nptr_len--;
88+
89+
if (!nptr_len)
90+
goto Return;
8891

8992
/*
9093
* Sign
@@ -94,24 +97,35 @@ int git__strntol64(int64_t *result, const char *nptr, size_t nptr_len, const cha
9497
neg = 1;
9598

9699
/*
97-
* Base
100+
* Automatically detect the base if none was given to us.
101+
* Right now, we assume that a number starting with '0x'
102+
* is hexadecimal and a number starting with '0' is
103+
* octal.
98104
*/
99105
if (base == 0) {
100106
if (*p != '0')
101107
base = 10;
102-
else {
108+
else if (nptr_len > 2 && (p[1] == 'x' || p[1] == 'X'))
109+
base = 16;
110+
else
103111
base = 8;
104-
if (p[1] == 'x' || p[1] == 'X') {
105-
p += 2;
106-
base = 16;
107-
}
108-
}
109-
} else if (base == 16 && *p == '0') {
110-
if (p[1] == 'x' || p[1] == 'X')
111-
p += 2;
112-
} else if (base < 0 || 36 < base)
112+
}
113+
114+
if (base < 0 || 36 < base)
113115
goto Return;
114116

117+
/*
118+
* Skip prefix of '0x'-prefixed hexadecimal numbers. There is no
119+
* need to do the same for '0'-prefixed octal numbers as a
120+
* leading '0' does not have any impact. Also, if we skip a
121+
* leading '0' in such a string, then we may end up with no
122+
* digits left and produce an error later on which isn't one.
123+
*/
124+
if (base == 16 && nptr_len > 2 && p[0] == '0' && (p[1] == 'x' || p[1] == 'X')) {
125+
p += 2;
126+
nptr_len -= 2;
127+
}
128+
115129
/*
116130
* Non-empty sequence of digits
117131
*/

‎tests/core/strtol.c‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,28 @@ void test_core_strtol__int64(void)
6464
assert_l64_fails("-0x8000000000000001", 16);
6565
}
6666

67+
void test_core_strtol__base_autodetection(void)
68+
{
69+
assert_l64_parses("0", 0, 0);
70+
assert_l64_parses("00", 0, 0);
71+
assert_l64_parses("0x", 0, 0);
72+
assert_l64_parses("0foobar", 0, 0);
73+
assert_l64_parses("07", 7, 0);
74+
assert_l64_parses("017", 15, 0);
75+
assert_l64_parses("0x8", 8, 0);
76+
assert_l64_parses("0x18", 24, 0);
77+
}
78+
79+
void test_core_strtol__buffer_length_with_autodetection_truncates(void)
80+
{
81+
int64_t i64;
82+
83+
cl_git_pass(git__strntol64(&i64, "011", 2, NULL, 0));
84+
cl_assert_equal_i(i64, 1);
85+
cl_git_pass(git__strntol64(&i64, "0x11", 3, NULL, 0));
86+
cl_assert_equal_i(i64, 1);
87+
}
88+
6789
void test_core_strtol__buffer_length_truncates(void)
6890
{
6991
int32_t i32;
@@ -76,6 +98,16 @@ void test_core_strtol__buffer_length_truncates(void)
7698
cl_assert_equal_i(i64, 1);
7799
}
78100

101+
void test_core_strtol__buffer_length_with_leading_ws_truncates(void)
102+
{
103+
int64_t i64;
104+
105+
cl_git_fail(git__strntol64(&i64, " 1", 1, NULL, 10));
106+
107+
cl_git_pass(git__strntol64(&i64, " 11", 2, NULL, 10));
108+
cl_assert_equal_i(i64, 1);
109+
}
110+
79111
void test_core_strtol__error_message_cuts_off(void)
80112
{
81113
assert_l32_fails("2147483657foobar", 10);

‎tests/object/tree/parse.c‎

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
#include "clar_libgit2.h"
2+
#include "tree.h"
3+
#include "object.h"
4+
5+
#define OID1_HEX \
6+
"\xae\x90\xf1\x2e\xea\x69\x97\x29\xed\x24" \
7+
"\x55\x5e\x40\xb9\xfd\x66\x9d\xa1\x2a\x12"
8+
#define OID1_STR "ae90f12eea699729ed24555e40b9fd669da12a12"
9+
10+
#define OID2_HEX \
11+
"\xe8\xbf\xe5\xaf\x39\x57\x9a\x7e\x48\x98" \
12+
"\xbb\x23\xf3\xa7\x6a\x72\xc3\x68\xce\xe6"
13+
#define OID2_STR "e8bfe5af39579a7e4898bb23f3a76a72c368cee6"
14+
15+
typedef struct {
16+
const char *filename;
17+
uint16_t attr;
18+
const char *oid;
19+
} expected_entry;
20+
21+
static void assert_tree_parses(const char *data, size_t datalen,
22+
expected_entry *expected_entries, size_t expected_nentries)
23+
{
24+
git_tree *tree;
25+
size_t n;
26+
27+
if (!datalen)
28+
datalen = strlen(data);
29+
cl_git_pass(git_object__from_raw((git_object **) &tree, data, datalen, GIT_OBJ_TREE));
30+
31+
cl_assert_equal_i(git_tree_entrycount(tree), expected_nentries);
32+
33+
for (n = 0; n < expected_nentries; n++) {
34+
expected_entry *expected = expected_entries + n;
35+
const git_tree_entry *entry;
36+
git_oid oid;
37+
38+
cl_git_pass(git_oid_fromstr(&oid, expected->oid));
39+
40+
cl_assert(entry = git_tree_entry_byname(tree, expected->filename));
41+
cl_assert_equal_s(expected->filename, entry->filename);
42+
cl_assert_equal_i(expected->attr, entry->attr);
43+
cl_assert_equal_oid(&oid, entry->oid);
44+
}
45+
46+
git_object_free(&tree->object);
47+
}
48+
49+
static void assert_tree_fails(const char *data, size_t datalen)
50+
{
51+
git_object *object;
52+
if (!datalen)
53+
datalen = strlen(data);
54+
cl_git_fail(git_object__from_raw(&object, data, datalen, GIT_OBJ_TREE));
55+
}
56+
57+
void test_object_tree_parse__single_blob_parses(void)
58+
{
59+
expected_entry entries[] = {
60+
{ "foo", 0100644, OID1_STR },
61+
};
62+
const char data[] = "100644 foo\x00" OID1_HEX;
63+
64+
assert_tree_parses(data, ARRAY_SIZE(data) - 1, entries, ARRAY_SIZE(entries));
65+
}
66+
67+
void test_object_tree_parse__single_tree_parses(void)
68+
{
69+
expected_entry entries[] = {
70+
{ "foo", 040000, OID1_STR },
71+
};
72+
const char data[] = "040000 foo\x00" OID1_HEX;
73+
74+
assert_tree_parses(data, ARRAY_SIZE(data) - 1, entries, ARRAY_SIZE(entries));
75+
}
76+
77+
void test_object_tree_parse__leading_filename_spaces_parse(void)
78+
{
79+
expected_entry entries[] = {
80+
{ " bar", 0100644, OID1_STR },
81+
};
82+
const char data[] = "100644 bar\x00" OID1_HEX;
83+
84+
assert_tree_parses(data, ARRAY_SIZE(data) - 1, entries, ARRAY_SIZE(entries));
85+
}
86+
87+
void test_object_tree_parse__multiple_entries_parse(void)
88+
{
89+
expected_entry entries[] = {
90+
{ "bar", 0100644, OID1_STR },
91+
{ "foo", 040000, OID2_STR },
92+
};
93+
const char data[] =
94+
"100644 bar\x00" OID1_HEX
95+
"040000 foo\x00" OID2_HEX;
96+
97+
assert_tree_parses(data, ARRAY_SIZE(data) - 1, entries, ARRAY_SIZE(entries));
98+
}
99+
100+
void test_object_tree_parse__invalid_mode_fails(void)
101+
{
102+
const char data[] = "10x644 bar\x00" OID1_HEX;
103+
assert_tree_fails(data, ARRAY_SIZE(data) - 1);
104+
}
105+
106+
void test_object_tree_parse__missing_mode_fails(void)
107+
{
108+
const char data[] = " bar\x00" OID1_HEX;
109+
assert_tree_fails(data, ARRAY_SIZE(data) - 1);
110+
}
111+
112+
void test_object_tree_parse__mode_doesnt_cause_oob_read(void)
113+
{
114+
const char data[] = "100644 bar\x00" OID1_HEX;
115+
assert_tree_fails(data, 2);
116+
/*
117+
* An oob-read would correctly parse the filename and
118+
* later fail to parse the OID with a different error
119+
* message
120+
*/
121+
cl_assert_equal_s(giterr_last()->message, "failed to parse tree: missing space after filemode");
122+
}
123+
124+
void test_object_tree_parse__unreasonably_large_mode_fails(void)
125+
{
126+
const char data[] = "10000000000000000000000000 bar\x00" OID1_HEX;
127+
assert_tree_fails(data, ARRAY_SIZE(data) - 1);
128+
}
129+
130+
void test_object_tree_parse__missing_filename_separator_fails(void)
131+
{
132+
const char data[] = "100644bar\x00" OID1_HEX;
133+
assert_tree_fails(data, ARRAY_SIZE(data) - 1);
134+
}
135+
136+
void test_object_tree_parse__missing_filename_terminator_fails(void)
137+
{
138+
const char data[] = "100644 bar" OID1_HEX;
139+
assert_tree_fails(data, ARRAY_SIZE(data) - 1);
140+
}
141+
142+
void test_object_tree_parse__empty_filename_fails(void)
143+
{
144+
const char data[] = "100644 \x00" OID1_HEX;
145+
assert_tree_fails(data, ARRAY_SIZE(data) - 1);
146+
}
147+
148+
void test_object_tree_parse__trailing_garbage_fails(void)
149+
{
150+
const char data[] = "100644 bar\x00" OID1_HEX "x";
151+
assert_tree_fails(data, ARRAY_SIZE(data) - 1);
152+
}
153+
154+
void test_object_tree_parse__leading_space_fails(void)
155+
{
156+
const char data[] = " 100644 bar\x00" OID1_HEX;
157+
assert_tree_fails(data, ARRAY_SIZE(data) - 1);
158+
}
159+
160+
void test_object_tree_parse__truncated_oid_fails(void)
161+
{
162+
const char data[] = " 100644 bar\x00" OID1_HEX;
163+
assert_tree_fails(data, ARRAY_SIZE(data) - 2);
164+
}

0 commit comments

Comments
 (0)