On Mon, Feb 5, 2024 at 9:23 AM Chandra Pratap via GitGitGadget <gitgitgadget@xxxxxxxxx> wrote: > > From: Chandra Pratap <chandrapratap3519@xxxxxxxxx> > > Signed-off-by: Chandra Pratap <chandrapratap3519@xxxxxxxxx> > --- > commit.c: ensure strchrnul() doesn't scan beyond range > > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1652%2FChand-ra%2Fstrchrnul-v1 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1652/Chand-ra/strchrnul-v1 > Pull-Request: https://github.com/gitgitgadget/git/pull/1652 > > commit.c | 8 +------- > 1 file changed, 1 insertion(+), 7 deletions(-) > > diff --git a/commit.c b/commit.c > index ef679a0b939..a65b8e92e94 100644 > --- a/commit.c > +++ b/commit.c > @@ -1743,15 +1743,9 @@ const char *find_header_mem(const char *msg, size_t len, > int key_len = strlen(key); > const char *line = msg; > > - /* > - * NEEDSWORK: It's possible for strchrnul() to scan beyond the range > - * given by len. However, current callers are safe because they compute > - * len by scanning a NUL-terminated block of memory starting at msg. > - * Nonetheless, it would be better to ensure the function does not look > - * at msg beyond the len provided by the caller. > - */ > while (line && line < msg + len) { > const char *eol = strchrnul(line, '\n'); > + assert(eol - line <= len); I don't think this is sufficient to address the NEEDSWORK. `assert` is only active in debug builds, and strchrnul would have already potentially exceeded the bounds of its memory by the time this check is happening. We'd need a safe version of strchrnul that took the maximum length and never exceeded it. > > if (line == eol) > return NULL; > > base-commit: a54a84b333adbecf7bc4483c0e36ed5878cac17b > -- > gitgitgadget >
