aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJunio C Hamano <gitster@pobox.com>2025-11-30 18:31:41 -0800
committerJunio C Hamano <gitster@pobox.com>2025-11-30 18:31:41 -0800
commitaea8cc3a10c325a22a75e2d4f582db959d3854ae (patch)
treeda952c26ede073dc3750d4fe2b5ee10bacae3464
parent6912d80f55ad6d6598c9c6986c1035a51784a836 (diff)
parenta031b6181a1e1ee6768d19d6a03b031b6e9004e9 (diff)
downloadgit-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--Makefile1
-rw-r--r--cache-tree.c50
-rw-r--r--compat/mmap.c2
-rw-r--r--fsck.c71
-rw-r--r--meson.build8
-rw-r--r--pack-bitmap.c29
-rw-r--r--t/test-lib.sh1
7 files changed, 122 insertions, 40 deletions
diff --git a/Makefile b/Makefile
index 2a67546154..237b56fc9d 100644
--- a/Makefile
+++ b/Makefile
@@ -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;
diff --git a/fsck.c b/fsck.c
index 341e100d24..8e8083e7c6 100644
--- a/fsck.c
+++ b/fsck.c
@@ -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