Skip to content

Commit 5096d49

Browse files
peffgitster
authored andcommitted
convert trivial sprintf / strcpy calls to xsnprintf
We sometimes sprintf into fixed-size buffers when we know that the buffer is large enough to fit the input (either because it's a constant, or because it's numeric input that is bounded in size). Likewise with strcpy of constant strings. However, these sites make it hard to audit sprintf and strcpy calls for buffer overflows, as a reader has to cross-reference the size of the array with the input. Let's use xsnprintf instead, which communicates to a reader that we don't expect this to overflow (and catches the mistake in case we do). Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent db85a8a commit 5096d49

20 files changed

+52
-47
lines changed

archive-tar.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,7 @@ static int write_global_extended_header(struct archiver_args *args)
301301
memset(&header, 0, sizeof(header));
302302
*header.typeflag = TYPEFLAG_GLOBAL_HEADER;
303303
mode = 0100666;
304-
strcpy(header.name, "pax_global_header");
304+
xsnprintf(header.name, sizeof(header.name), "pax_global_header");
305305
prepare_header(args, &header, mode, ext_header.len);
306306
write_blocked(&header, sizeof(header));
307307
write_blocked(ext_header.buf, ext_header.len);

builtin/gc.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid)
194194
return NULL;
195195

196196
if (gethostname(my_host, sizeof(my_host)))
197-
strcpy(my_host, "unknown");
197+
xsnprintf(my_host, sizeof(my_host), "unknown");
198198

199199
pidfile_path = git_pathdup("gc.pid");
200200
fd = hold_lock_file_for_update(&lock, pidfile_path,

builtin/init-db.c

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,8 @@ static int create_default_files(const char *template_path)
262262
}
263263

264264
/* This forces creation of new config file */
265-
sprintf(repo_version_string, "%d", GIT_REPO_VERSION);
265+
xsnprintf(repo_version_string, sizeof(repo_version_string),
266+
"%d", GIT_REPO_VERSION);
266267
git_config_set("core.repositoryformatversion", repo_version_string);
267268

268269
path[len] = 0;
@@ -414,13 +415,13 @@ int init_db(const char *template_dir, unsigned int flags)
414415
*/
415416
if (shared_repository < 0)
416417
/* force to the mode value */
417-
sprintf(buf, "0%o", -shared_repository);
418+
xsnprintf(buf, sizeof(buf), "0%o", -shared_repository);
418419
else if (shared_repository == PERM_GROUP)
419-
sprintf(buf, "%d", OLD_PERM_GROUP);
420+
xsnprintf(buf, sizeof(buf), "%d", OLD_PERM_GROUP);
420421
else if (shared_repository == PERM_EVERYBODY)
421-
sprintf(buf, "%d", OLD_PERM_EVERYBODY);
422+
xsnprintf(buf, sizeof(buf), "%d", OLD_PERM_EVERYBODY);
422423
else
423-
die("oops");
424+
die("BUG: invalid value for shared_repository");
424425
git_config_set("core.sharedrepository", buf);
425426
git_config_set("receive.denyNonFastforwards", "true");
426427
}

builtin/ls-tree.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,12 +96,13 @@ static int show_tree(const unsigned char *sha1, struct strbuf *base,
9696
if (!strcmp(type, blob_type)) {
9797
unsigned long size;
9898
if (sha1_object_info(sha1, &size) == OBJ_BAD)
99-
strcpy(size_text, "BAD");
99+
xsnprintf(size_text, sizeof(size_text),
100+
"BAD");
100101
else
101-
snprintf(size_text, sizeof(size_text),
102-
"%lu", size);
102+
xsnprintf(size_text, sizeof(size_text),
103+
"%lu", size);
103104
} else
104-
strcpy(size_text, "-");
105+
xsnprintf(size_text, sizeof(size_text), "-");
105106
printf("%06o %s %s %7s\t", mode, type,
106107
find_unique_abbrev(sha1, abbrev),
107108
size_text);

builtin/merge-index.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ static int merge_entry(int pos, const char *path)
2323
break;
2424
found++;
2525
strcpy(hexbuf[stage], sha1_to_hex(ce->sha1));
26-
sprintf(ownbuf[stage], "%o", ce->ce_mode);
26+
xsnprintf(ownbuf[stage], sizeof(ownbuf[stage]), "%o", ce->ce_mode);
2727
arguments[stage] = hexbuf[stage];
2828
arguments[stage + 4] = ownbuf[stage];
2929
} while (++pos < active_nr);

builtin/merge-recursive.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ static const char *better_branch_name(const char *branch)
1414

1515
if (strlen(branch) != 40)
1616
return branch;
17-
sprintf(githead_env, "GITHEAD_%s", branch);
17+
xsnprintf(githead_env, sizeof(githead_env), "GITHEAD_%s", branch);
1818
name = getenv(githead_env);
1919
return name ? name : branch;
2020
}

builtin/read-tree.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ static int debug_merge(const struct cache_entry * const *stages,
9090
debug_stage("index", stages[0], o);
9191
for (i = 1; i <= o->merge_size; i++) {
9292
char buf[24];
93-
sprintf(buf, "ent#%d", i);
93+
xsnprintf(buf, sizeof(buf), "ent#%d", i);
9494
debug_stage(buf, stages[i], o);
9595
}
9696
return 0;

builtin/unpack-file.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ static char *create_temp_file(unsigned char *sha1)
1212
if (!buf || type != OBJ_BLOB)
1313
die("unable to read blob object %s", sha1_to_hex(sha1));
1414

15-
strcpy(path, ".merge_file_XXXXXX");
15+
xsnprintf(path, sizeof(path), ".merge_file_XXXXXX");
1616
fd = xmkstemp(path);
1717
if (write_in_full(fd, buf, size) != size)
1818
die_errno("unable to write temp-file");

compat/mingw.c

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2133,9 +2133,11 @@ int uname(struct utsname *buf)
21332133
{
21342134
DWORD v = GetVersion();
21352135
memset(buf, 0, sizeof(*buf));
2136-
strcpy(buf->sysname, "Windows");
2137-
sprintf(buf->release, "%u.%u", v & 0xff, (v >> 8) & 0xff);
2136+
xsnprintf(buf->sysname, sizeof(buf->sysname), "Windows");
2137+
xsnprintf(buf->release, sizeof(buf->release),
2138+
"%u.%u", v & 0xff, (v >> 8) & 0xff);
21382139
/* assuming NT variants only.. */
2139-
sprintf(buf->version, "%u", (v >> 16) & 0x7fff);
2140+
xsnprintf(buf->version, sizeof(buf->version),
2141+
"%u", (v >> 16) & 0x7fff);
21402142
return 0;
21412143
}

compat/winansi.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,7 @@ void winansi_init(void)
539539
return;
540540

541541
/* create a named pipe to communicate with the console thread */
542-
sprintf(name, "\\\\.\\pipe\\winansi%lu", GetCurrentProcessId());
542+
xsnprintf(name, sizeof(name), "\\\\.\\pipe\\winansi%lu", GetCurrentProcessId());
543543
hwrite = CreateNamedPipe(name, PIPE_ACCESS_OUTBOUND,
544544
PIPE_TYPE_BYTE | PIPE_WAIT, 1, BUFFER_SIZE, 0, 0, NULL);
545545
if (hwrite == INVALID_HANDLE_VALUE)

0 commit comments

Comments
 (0)