diff options
| author | Junio C Hamano <gitster@pobox.com> | 2025-11-30 18:31:41 -0800 |
|---|---|---|
| committer | Junio C Hamano <gitster@pobox.com> | 2025-11-30 18:31:41 -0800 |
| commit | aea8cc3a10c325a22a75e2d4f582db959d3854ae (patch) | |
| tree | da952c26ede073dc3750d4fe2b5ee10bacae3464 | |
| parent | 6912d80f55ad6d6598c9c6986c1035a51784a836 (diff) | |
| parent | a031b6181a1e1ee6768d19d6a03b031b6e9004e9 (diff) | |
| download | git-aea8cc3a10c325a22a75e2d4f582db959d3854ae.tar.gz | |
Merge branch 'jk/asan-bonanza'
Various issues detected by Asan have been corrected.
* jk/asan-bonanza:
t: enable ASan's strict_string_checks option
fsck: avoid parse_timestamp() on buffer that isn't NUL-terminated
fsck: remove redundant date timestamp check
fsck: avoid strcspn() in fsck_ident()
fsck: assert newline presence in fsck_ident()
cache-tree: avoid strtol() on non-string buffer
Makefile: turn on NO_MMAP when building with ASan
pack-bitmap: handle name-hash lookups in incremental bitmaps
compat/mmap: mark unused argument in git_munmap()
| -rw-r--r-- | Makefile | 1 | ||||
| -rw-r--r-- | cache-tree.c | 50 | ||||
| -rw-r--r-- | compat/mmap.c | 2 | ||||
| -rw-r--r-- | fsck.c | 71 | ||||
| -rw-r--r-- | meson.build | 8 | ||||
| -rw-r--r-- | pack-bitmap.c | 29 | ||||
| -rw-r--r-- | t/test-lib.sh | 1 |
7 files changed, 122 insertions, 40 deletions
@@ -1588,6 +1588,7 @@ SANITIZE_LEAK = YesCompiledWithIt endif ifneq ($(filter address,$(SANITIZERS)),) NO_REGEX = NeededForASAN +NO_MMAP = NeededForASAN SANITIZE_ADDRESS = YesCompiledWithIt endif endif diff --git a/cache-tree.c b/cache-tree.c index 2aba47060e..2d8947b518 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -548,12 +548,41 @@ void cache_tree_write(struct strbuf *sb, struct cache_tree *root) trace2_region_leave("cache_tree", "write", the_repository); } +static int parse_int(const char **ptr, unsigned long *len_p, int *out) +{ + const char *s = *ptr; + unsigned long len = *len_p; + int ret = 0; + int sign = 1; + + while (len && *s == '-') { + sign *= -1; + s++; + len--; + } + + while (len) { + if (!isdigit(*s)) + break; + ret *= 10; + ret += *s - '0'; + s++; + len--; + } + + if (s == *ptr) + return -1; + + *ptr = s; + *len_p = len; + *out = sign * ret; + return 0; +} + static struct cache_tree *read_one(const char **buffer, unsigned long *size_p) { const char *buf = *buffer; unsigned long size = *size_p; - const char *cp; - char *ep; struct cache_tree *it; int i, subtree_nr; const unsigned rawsz = the_hash_algo->rawsz; @@ -569,19 +598,14 @@ static struct cache_tree *read_one(const char **buffer, unsigned long *size_p) buf++; size--; it = cache_tree(); - cp = buf; - it->entry_count = strtol(cp, &ep, 10); - if (cp == ep) + if (parse_int(&buf, &size, &it->entry_count) < 0) goto free_return; - cp = ep; - subtree_nr = strtol(cp, &ep, 10); - if (cp == ep) + if (!size || *buf != ' ') goto free_return; - while (size && *buf && *buf != '\n') { - size--; - buf++; - } - if (!size) + buf++; size--; + if (parse_int(&buf, &size, &subtree_nr) < 0) + goto free_return; + if (!size || *buf != '\n') goto free_return; buf++; size--; if (0 <= it->entry_count) { diff --git a/compat/mmap.c b/compat/mmap.c index 2fe1c7732e..1a118711f7 100644 --- a/compat/mmap.c +++ b/compat/mmap.c @@ -38,7 +38,7 @@ void *git_mmap(void *start, size_t length, int prot, int flags, int fd, off_t of return start; } -int git_munmap(void *start, size_t length) +int git_munmap(void *start, size_t length UNUSED) { free(start); return 0; @@ -860,31 +860,60 @@ static int verify_headers(const void *data, unsigned long size, FSCK_MSG_UNTERMINATED_HEADER, "unterminated header"); } -static int fsck_ident(const char **ident, +static timestamp_t parse_timestamp_from_buf(const char **start, const char *end) +{ + const char *p = *start; + char buf[24]; /* big enough for 2^64 */ + size_t i = 0; + + while (p < end && isdigit(*p)) { + if (i >= ARRAY_SIZE(buf) - 1) + return TIME_MAX; + buf[i++] = *p++; + } + buf[i] = '\0'; + *start = p; + return parse_timestamp(buf, NULL, 10); +} + +static int fsck_ident(const char **ident, const char *ident_end, const struct object_id *oid, enum object_type type, struct fsck_options *options) { const char *p = *ident; - char *end; + const char *nl; - *ident = strchrnul(*ident, '\n'); - if (**ident == '\n') - (*ident)++; + nl = memchr(p, '\n', ident_end - p); + if (!nl) + BUG("verify_headers() should have made sure we have a newline"); + *ident = nl + 1; if (*p == '<') return report(options, oid, type, FSCK_MSG_MISSING_NAME_BEFORE_EMAIL, "invalid author/committer line - missing space before email"); - p += strcspn(p, "<>\n"); - if (*p == '>') - return report(options, oid, type, FSCK_MSG_BAD_NAME, "invalid author/committer line - bad name"); - if (*p != '<') - return report(options, oid, type, FSCK_MSG_MISSING_EMAIL, "invalid author/committer line - missing email"); + for (;;) { + if (p >= ident_end || *p == '\n') + return report(options, oid, type, FSCK_MSG_MISSING_EMAIL, "invalid author/committer line - missing email"); + if (*p == '>') + return report(options, oid, type, FSCK_MSG_BAD_NAME, "invalid author/committer line - bad name"); + if (*p == '<') + break; /* end of name, beginning of email */ + + /* otherwise, skip past arbitrary name char */ + p++; + } if (p[-1] != ' ') return report(options, oid, type, FSCK_MSG_MISSING_SPACE_BEFORE_EMAIL, "invalid author/committer line - missing space before email"); - p++; - p += strcspn(p, "<>\n"); - if (*p != '>') - return report(options, oid, type, FSCK_MSG_BAD_EMAIL, "invalid author/committer line - bad email"); - p++; + p++; /* skip past '<' we found */ + for (;;) { + if (p >= ident_end || *p == '<' || *p == '\n') + return report(options, oid, type, FSCK_MSG_BAD_EMAIL, "invalid author/committer line - bad email"); + if (*p == '>') + break; /* end of email */ + + /* otherwise, skip past arbitrary email char */ + p++; + } + p++; /* skip past '>' we found */ if (*p != ' ') return report(options, oid, type, FSCK_MSG_MISSING_SPACE_BEFORE_DATE, "invalid author/committer line - missing space before date"); p++; @@ -904,11 +933,11 @@ static int fsck_ident(const char **ident, "invalid author/committer line - bad date"); if (*p == '0' && p[1] != ' ') return report(options, oid, type, FSCK_MSG_ZERO_PADDED_DATE, "invalid author/committer line - zero-padded date"); - if (date_overflows(parse_timestamp(p, &end, 10))) + if (date_overflows(parse_timestamp_from_buf(&p, ident_end))) return report(options, oid, type, FSCK_MSG_BAD_DATE_OVERFLOW, "invalid author/committer line - date causes integer overflow"); - if ((end == p || *end != ' ')) + if (*p != ' ') return report(options, oid, type, FSCK_MSG_BAD_DATE, "invalid author/committer line - bad date"); - p = end + 1; + p++; if ((*p != '+' && *p != '-') || !isdigit(p[1]) || !isdigit(p[2]) || @@ -958,7 +987,7 @@ static int fsck_commit(const struct object_id *oid, author_count = 0; while (buffer < buffer_end && skip_prefix(buffer, "author ", &buffer)) { author_count++; - err = fsck_ident(&buffer, oid, OBJ_COMMIT, options); + err = fsck_ident(&buffer, buffer_end, oid, OBJ_COMMIT, options); if (err) return err; } @@ -970,7 +999,7 @@ static int fsck_commit(const struct object_id *oid, return err; if (buffer >= buffer_end || !skip_prefix(buffer, "committer ", &buffer)) return report(options, oid, OBJ_COMMIT, FSCK_MSG_MISSING_COMMITTER, "invalid format - expected 'committer' line"); - err = fsck_ident(&buffer, oid, OBJ_COMMIT, options); + err = fsck_ident(&buffer, buffer_end, oid, OBJ_COMMIT, options); if (err) return err; if (memchr(buffer_begin, '\0', size)) { @@ -1065,7 +1094,7 @@ int fsck_tag_standalone(const struct object_id *oid, const char *buffer, goto done; } else - ret = fsck_ident(&buffer, oid, OBJ_TAG, options); + ret = fsck_ident(&buffer, buffer_end, oid, OBJ_TAG, options); if (buffer < buffer_end && (skip_prefix(buffer, "gpgsig ", &buffer) || skip_prefix(buffer, "gpgsig-sha256 ", &buffer))) { eol = memchr(buffer, '\n', buffer_end - buffer); diff --git a/meson.build b/meson.build index 1f95a06edb..f1b3615659 100644 --- a/meson.build +++ b/meson.build @@ -1411,12 +1411,18 @@ if host_machine.system() == 'windows' libgit_c_args += '-DUSE_WIN32_MMAP' else checkfuncs += { - 'mmap' : ['mmap.c'], # provided by compat/mingw.c. 'unsetenv' : ['unsetenv.c'], # provided by compat/mingw.c. 'getpagesize' : [], } + + if get_option('b_sanitize').contains('address') + libgit_c_args += '-DNO_MMAP' + libgit_sources += 'compat/mmap.c' + else + checkfuncs += { 'mmap': ['mmap.c'] } + endif endif foreach func, impls : checkfuncs diff --git a/pack-bitmap.c b/pack-bitmap.c index 291e1a9cf4..8ca79725b1 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -213,6 +213,28 @@ static uint32_t bitmap_num_objects(struct bitmap_index *index) return index->pack->num_objects; } +static uint32_t bitmap_name_hash(struct bitmap_index *index, uint32_t pos) +{ + if (bitmap_is_midx(index)) { + while (index && pos < index->midx->num_objects_in_base) { + ASSERT(bitmap_is_midx(index)); + index = index->base; + } + + if (!index) + BUG("NULL base bitmap for object position: %"PRIu32, pos); + + pos -= index->midx->num_objects_in_base; + if (pos >= index->midx->num_objects) + BUG("out-of-bounds midx bitmap object at %"PRIu32, pos); + } + + if (!index->hashes) + return 0; + + return get_be32(index->hashes + pos); +} + static struct repository *bitmap_repo(struct bitmap_index *bitmap_git) { if (bitmap_is_midx(bitmap_git)) @@ -1724,8 +1746,7 @@ static void show_objects_for_type( pack = bitmap_git->pack; } - if (bitmap_git->hashes) - hash = get_be32(bitmap_git->hashes + index_pos); + hash = bitmap_name_hash(bitmap_git, index_pos); show_reach(&oid, object_type, 0, hash, pack, ofs, payload); } @@ -3124,8 +3145,8 @@ uint32_t *create_bitmap_mapping(struct bitmap_index *bitmap_git, if (oe) { reposition[i] = oe_in_pack_pos(mapping, oe) + 1; - if (bitmap_git->hashes && !oe->hash) - oe->hash = get_be32(bitmap_git->hashes + index_pos); + if (!oe->hash) + oe->hash = bitmap_name_hash(bitmap_git, index_pos); } } diff --git a/t/test-lib.sh b/t/test-lib.sh index ef0ab7ec2d..0fb76f7d11 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -77,6 +77,7 @@ prepend_var GIT_SAN_OPTIONS : strip_path_prefix="$GIT_BUILD_DIR/" # want that one to complain to stderr). prepend_var ASAN_OPTIONS : $GIT_SAN_OPTIONS prepend_var ASAN_OPTIONS : detect_leaks=0 +prepend_var ASAN_OPTIONS : strict_string_checks=1 export ASAN_OPTIONS prepend_var LSAN_OPTIONS : $GIT_SAN_OPTIONS |
