git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 15/18] convert unchecked snprintf into xsnprintf

From
Jeff King <peff@peff.net>
Date
Mar 28, 2017, 19:46 UTC
Message-ID
<20170328194656.d4sgj67psmpnnmua@sigill.intra.peff.net>
In-Reply-To
<20170328194255.vf7nfzzmmzxsbn36@sigill.intra.peff.net>

These calls to snprintf should always succeed, because their input is small and fixed. Let's use xsnprintf to make sure this is the case (and to make auditing for actual truncation easier).

These could be candidates for turning into heap buffers, but they fall into a few broad categories that make it not worth doing:

  - formatting single numbers is simple enough that we can
    see the result should fit
  - the size of a sha1 is likewise well-known, and I didn't
    want to cause unnecessary conflicts with the ongoing
    process to convert these constants to GIT_MAX_HEXSZ
  - the interface for curl_errorstr is dictated by curl
Signed-off-by: Jeff King <peff@peff.net>
---
 grep.c      |  4 ++--
 http.c      | 10 +++++-----
 imap-send.c |  2 +-
 sha1_file.c |  4 ++--
 submodule.c |  2 +-
 5 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/grep.c b/grep.c
index 0dbdc1d00..39b4b60d2 100644
--- a/grep.c
+++ b/grep.c
@@ -1164,7 +1164,7 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 	}
 	if (opt->linenum) {
 		char buf[32];
-		snprintf(buf, sizeof(buf), "%d", lno);
+		xsnprintf(buf, sizeof(buf), "%d", lno);
 		output_color(opt, buf, strlen(buf), opt->color_lineno);
 		output_sep(opt, sign);
 	}
@@ -1651,7 +1651,7 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle
 				     opt->color_filename);
 			output_sep(opt, ':');
 		}
-		snprintf(buf, sizeof(buf), "%u\n", count);
+		xsnprintf(buf, sizeof(buf), "%u\n", count);
 		opt->output(opt, buf, strlen(buf));
 		return 1;
 	}
diff --git a/http.c b/http.c
index 96d84bbed..8d94e2c63 100644
--- a/http.c
+++ b/http.c
@@ -1366,9 +1366,9 @@ static int handle_curl_result(struct slot_results *results)
 		 * FAILONERROR it is lost, so we can give only the numeric
 		 * status code.
 		 */
-		snprintf(curl_errorstr, sizeof(curl_errorstr),
-			 "The requested URL returned error: %ld",
-			 results->http_code);
+		xsnprintf(curl_errorstr, sizeof(curl_errorstr),
+			  "The requested URL returned error: %ld",
+			  results->http_code);
 	}
 
 	if (results->curl_result == CURLE_OK) {
@@ -1410,8 +1410,8 @@ int run_one_slot(struct active_request_slot *slot,
 {
 	slot->results = results;
 	if (!start_active_slot(slot)) {
-		snprintf(curl_errorstr, sizeof(curl_errorstr),
-			 "failed to start HTTP request");
+		xsnprintf(curl_errorstr, sizeof(curl_errorstr),
+			  "failed to start HTTP request");
 		return HTTP_START_FAILED;
 	}
 
diff --git a/imap-send.c b/imap-send.c
index 5c7e27a89..857591660 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -964,7 +964,7 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, char *f
 		int gai;
 		char portstr[6];
 
-		snprintf(portstr, sizeof(portstr), "%d", srvc->port);
+		xsnprintf(portstr, sizeof(portstr), "%d", srvc->port);
 
 		memset(&hints, 0, sizeof(hints));
 		hints.ai_socktype = SOCK_STREAM;
diff --git a/sha1_file.c b/sha1_file.c
index 71063890f..43990dec7 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -3762,8 +3762,8 @@ static int for_each_file_in_obj_subdir(int subdir_nr,
 			char hex[GIT_SHA1_HEXSZ+1];
 			struct object_id oid;
 
-			snprintf(hex, sizeof(hex), "%02x%s",
-				 subdir_nr, de->d_name);
+			xsnprintf(hex, sizeof(hex), "%02x%s",
+				  subdir_nr, de->d_name);
 			if (!get_oid_hex(hex, &oid)) {
 				if (obj_cb) {
 					r = obj_cb(&oid, path->buf, data);
diff --git a/submodule.c b/submodule.c
index 3200b7bb2..e11ea7d0a 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1221,7 +1221,7 @@ static int find_first_merges(struct object_array *result, const char *path,
 	memset(&rev_opts, 0, sizeof(rev_opts));
 
 	/* get all revisions that merge commit a */
-	snprintf(merged_revision, sizeof(merged_revision), "^%s",
+	xsnprintf(merged_revision, sizeof(merged_revision), "^%s",
 			oid_to_hex(&a->object.oid));
 	init_revisions(&revs, NULL);
 	rev_opts.submodule = path;
-- 
2.12.2.845.g55fcf8b10
Previous: Jeff KingNext: Jeff King
Message 18 of 30 in “snprintf cleanups”
  1. 0/18 snprintf cleanupsJeff King, Mar 28, 2017
  2. 01/18 do not check odb_mkstemp return value for errorsJeff King, Mar 28, 2017
  3. 02/18 odb_mkstemp: write filename into strbufJeff King, Mar 28, 2017
  4. 03/18 odb_mkstemp: use git_path_bufJeff King, Mar 28, 2017
  5. 05/18 tag: use strbuf to format tag headerJeff King, Mar 28, 2017
  6. 09/18 create_branch: move msg setup closer to point of useJeff King, Mar 28, 2017
  7. 08/18 avoid using mksnpath for refsJeff King, Mar 28, 2017
  8. 06/18 fetch: use heap buffer to format reflogJeff King, Mar 28, 2017
  9. 04/18 diff: avoid fixed-size buffer for patch-idsJeff King, Mar 28, 2017
  10. Jeff KingMar 28, 2017
  11. 13/18 replace unchecked snprintf calls with heap buffersJeff King, Mar 28, 2017
  12. 12/18 receive-pack: print --pack-header directly into argv arrayJeff King, Mar 28, 2017
  13. 07/18 avoid using fixed PATH_MAX buffers for refsJeff King, Mar 28, 2017
  14. Junio C HamanoApr 17, 2017
  15. Jeff KingApr 18, 2017
  16. Junio C HamanoApr 18, 2017
  17. 11/18 name-rev: replace static buffer with strbufJeff King, Mar 28, 2017
  18. 15/18 convert unchecked snprintf into xsnprintfJeff King, Mar 28, 2017
  19. 10/18 create_branch: use xstrfmt for reflog messageJeff King, Mar 28, 2017
  20. 14/18 combine-diff: replace malloc/snprintf with xstrfmtJeff King, Mar 28, 2017
  21. 17/18 gc: replace local buffer with git_pathJeff King, Mar 28, 2017
  22. 16/18 transport-helper: replace checked snprintf with xsnprintfJeff King, Mar 28, 2017
  23. 18/18 daemon: use an argv_array to exec childrenJeff King, Mar 28, 2017
  24. Junio C HamanoMar 28, 2017
  25. Jeff KingMar 29, 2017
  26. Junio C HamanoMar 29, 2017
  27. Jeff KingMar 30, 2017
  28. Junio C HamanoMar 30, 2017
  29. Jeff KingMar 30, 2017
  30. Makefile: detect errors in running spatchJeff King, Mar 29, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.