git.vger.kernel.org archive mirror
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).