Am 07.02.24 um 14:57 schrieb Chandra Pratap via GitGitGadget: > From: Chandra Pratap <chandrapratap3519@xxxxxxxxx> > > Signed-off-by: Chandra Pratap <chandrapratap3519@xxxxxxxxx> > --- > commit.c: ensure find_header_mem() doesn't scan beyond given range > > Thanks for the feedback, Kyle and René! I have update the patch to > actually solve the problem at hand but I am not very sure about the > resulting dropping of const-ness of 'eol' from this and how big of a > problem it might create (if any). I wonder if a custom strchrnul() is > the best solution to this after all. > > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1652%2FChand-ra%2Fstrchrnul-v2 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1652/Chand-ra/strchrnul-v2 > Pull-Request: https://github.com/gitgitgadget/git/pull/1652 > > Range-diff vs v1: > > 1: 1c62f6ee353 ! 1: dcb2de3faea commit.c: ensure strchrnul() doesn't scan beyond range > @@ Metadata > Author: Chandra Pratap <chandrapratap3519@xxxxxxxxx> > > ## Commit message ## > - commit.c: ensure strchrnul() doesn't scan beyond range > + commit.c: ensure find_header_mem() doesn't scan beyond given range > > Signed-off-by: Chandra Pratap <chandrapratap3519@xxxxxxxxx> > > @@ commit.c: const char *find_header_mem(const char *msg, size_t len, > - * at msg beyond the len provided by the caller. > - */ > while (line && line < msg + len) { > - const char *eol = strchrnul(line, '\n'); > -+ assert(eol - line <= len); > +- const char *eol = strchrnul(line, '\n'); > ++ char *eol = (char *) line; > ++ for (size_t i = 0; i < len && *eol && *eol != '\n'; i++) { > ++ eol++; > ++ } > > if (line == eol) > return NULL; > > > commit.c | 12 ++++-------- > 1 file changed, 4 insertions(+), 8 deletions(-) > > diff --git a/commit.c b/commit.c > index ef679a0b939..9a460b2fd6f 100644 > --- a/commit.c > +++ b/commit.c > @@ -1743,15 +1743,11 @@ 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'); > + char *eol = (char *) line; > + for (size_t i = 0; i < len && *eol && *eol != '\n'; i++) { > + eol++; > + } This uses the pointer eol only for reading, so you can keep it const. The loop starts counting from 0 to len for each line, which cannot be right. find_header_mem("headers\nfoo bar", 9, "foo", &len) would still return "bar" instead of NULL. You could initialize i to the offset of line within msg instead (i.e. i = line - msg). Or check eol < msg + len instead of i < len -- then you don't even need to introduce that separate counter. Style nit: We tend to omit curly braces if they contain only a single statement. > if (line == eol) > return NULL; > > base-commit: a54a84b333adbecf7bc4483c0e36ed5878cac17b
