All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Chandra Pratap via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: "René Scharfe" <l.s.r@web.de>,
	"Kyle Lippincott" <spectral@google.com>,
	"Chandra Pratap" <chandrapratap376@gmail.com>,
	"Chandra Pratap" <chandrapratap3519@gmail.com>
Subject: [PATCH v2] commit.c: ensure find_header_mem() doesn't scan beyond given range
Date: Wed, 07 Feb 2024 13:57:26 +0000	[thread overview]
Message-ID: <pull.1652.v2.git.1707314246530.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1652.git.1707153705840.gitgitgadget@gmail.com>

From: Chandra Pratap <chandrapratap3519@gmail.com>

Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
---
    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@gmail.com>
      
       ## 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@gmail.com>
      
     @@ 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++;
+		}
 
 		if (line == eol)
 			return NULL;

base-commit: a54a84b333adbecf7bc4483c0e36ed5878cac17b
-- 
gitgitgadget

  parent reply	other threads:[~2024-02-07 13:57 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-05 17:21 [PATCH] commit.c: ensure strchrnul() doesn't scan beyond range Chandra Pratap via GitGitGadget
2024-02-05 19:57 ` René Scharfe
2024-02-06 18:44   ` Junio C Hamano
2024-02-08  1:00   ` Jeff King
2024-02-08 18:31     ` René Scharfe
2024-02-08 19:48       ` Junio C Hamano
2024-02-08 19:52         ` Kyle Lippincott
2024-02-08 21:41         ` Jeff King
2024-02-08 21:44           ` Junio C Hamano
2024-02-06  1:41 ` Kyle Lippincott
2024-02-07 13:57 ` Chandra Pratap via GitGitGadget [this message]
2024-02-07 17:09   ` [PATCH v2] commit.c: ensure find_header_mem() doesn't scan beyond given range René Scharfe
2024-02-07 17:23     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.1652.v2.git.1707314246530.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=chandrapratap3519@gmail.com \
    --cc=chandrapratap376@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=l.s.r@web.de \
    --cc=spectral@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.