aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJeff King <peff@peff.net>2015-09-24 17:06:08 -0400
committerJunio C Hamano <gitster@pobox.com>2015-09-25 10:18:18 -0700
commit5096d4909f9b13c7a650d9dbb7c9702ea7413566 (patch)
tree07229a8952f2d6782f3064d9dfd1e7855e8e5269
parentdb85a8a9c2fb492d3cd528dbbcc52075c607cf79 (diff)
downloadgit-5096d4909f9b13c7a650d9dbb7c9702ea7413566.tar.gz
git-5096d4909f9b13c7a650d9dbb7c9702ea7413566.tar.xz
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 <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
-rw-r--r--archive-tar.c2
-rw-r--r--builtin/gc.c2
-rw-r--r--builtin/init-db.c11
-rw-r--r--builtin/ls-tree.c9
-rw-r--r--builtin/merge-index.c2
-rw-r--r--builtin/merge-recursive.c2
-rw-r--r--builtin/read-tree.c2
-rw-r--r--builtin/unpack-file.c2
-rw-r--r--compat/mingw.c8
-rw-r--r--compat/winansi.c2
-rw-r--r--connect.c2
-rw-r--r--convert.c3
-rw-r--r--daemon.c4
-rw-r--r--diff.c12
-rw-r--r--http-push.c2
-rw-r--r--http.c6
-rw-r--r--ll-merge.c12
-rw-r--r--refs.c8
-rw-r--r--sideband.c4
-rw-r--r--strbuf.c4
20 files changed, 52 insertions, 47 deletions
diff --git a/archive-tar.c b/archive-tar.c
index b6b30bb57..d543f93fc 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -301,7 +301,7 @@ static int write_global_extended_header(struct archiver_args *args)
memset(&header, 0, sizeof(header));
*header.typeflag = TYPEFLAG_GLOBAL_HEADER;
mode = 0100666;
- strcpy(header.name, "pax_global_header");
+ xsnprintf(header.name, sizeof(header.name), "pax_global_header");
prepare_header(args, &header, mode, ext_header.len);
write_blocked(&header, sizeof(header));
write_blocked(ext_header.buf, ext_header.len);
diff --git a/builtin/gc.c b/builtin/gc.c
index 0ad8d30b5..57584bc99 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -194,7 +194,7 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid)
return NULL;
if (gethostname(my_host, sizeof(my_host)))
- strcpy(my_host, "unknown");
+ xsnprintf(my_host, sizeof(my_host), "unknown");
pidfile_path = git_pathdup("gc.pid");
fd = hold_lock_file_for_update(&lock, pidfile_path,
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 69323e186..e7d0e31f4 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -262,7 +262,8 @@ static int create_default_files(const char *template_path)
}
/* This forces creation of new config file */
- sprintf(repo_version_string, "%d", GIT_REPO_VERSION);
+ xsnprintf(repo_version_string, sizeof(repo_version_string),
+ "%d", GIT_REPO_VERSION);
git_config_set("core.repositoryformatversion", repo_version_string);
path[len] = 0;
@@ -414,13 +415,13 @@ int init_db(const char *template_dir, unsigned int flags)
*/
if (shared_repository < 0)
/* force to the mode value */
- sprintf(buf, "0%o", -shared_repository);
+ xsnprintf(buf, sizeof(buf), "0%o", -shared_repository);
else if (shared_repository == PERM_GROUP)
- sprintf(buf, "%d", OLD_PERM_GROUP);
+ xsnprintf(buf, sizeof(buf), "%d", OLD_PERM_GROUP);
else if (shared_repository == PERM_EVERYBODY)
- sprintf(buf, "%d", OLD_PERM_EVERYBODY);
+ xsnprintf(buf, sizeof(buf), "%d", OLD_PERM_EVERYBODY);
else
- die("oops");
+ die("BUG: invalid value for shared_repository");
git_config_set("core.sharedrepository", buf);
git_config_set("receive.denyNonFastforwards", "true");
}
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index 3b04a0f08..0e30d8623 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -96,12 +96,13 @@ static int show_tree(const unsigned char *sha1, struct strbuf *base,
if (!strcmp(type, blob_type)) {
unsigned long size;
if (sha1_object_info(sha1, &size) == OBJ_BAD)
- strcpy(size_text, "BAD");
+ xsnprintf(size_text, sizeof(size_text),
+ "BAD");
else
- snprintf(size_text, sizeof(size_text),
- "%lu", size);
+ xsnprintf(size_text, sizeof(size_text),
+ "%lu", size);
} else
- strcpy(size_text, "-");
+ xsnprintf(size_text, sizeof(size_text), "-");
printf("%06o %s %s %7s\t", mode, type,
find_unique_abbrev(sha1, abbrev),
size_text);
diff --git a/builtin/merge-index.c b/builtin/merge-index.c
index 1a1eafa6f..1d6611191 100644
--- a/builtin/merge-index.c
+++ b/builtin/merge-index.c
@@ -23,7 +23,7 @@ static int merge_entry(int pos, const char *path)
break;
found++;
strcpy(hexbuf[stage], sha1_to_hex(ce->sha1));
- sprintf(ownbuf[stage], "%o", ce->ce_mode);
+ xsnprintf(ownbuf[stage], sizeof(ownbuf[stage]), "%o", ce->ce_mode);
arguments[stage] = hexbuf[stage];
arguments[stage + 4] = ownbuf[stage];
} while (++pos < active_nr);
diff --git a/builtin/merge-recursive.c b/builtin/merge-recursive.c
index a90f28f34..491efd556 100644
--- a/builtin/merge-recursive.c
+++ b/builtin/merge-recursive.c
@@ -14,7 +14,7 @@ static const char *better_branch_name(const char *branch)
if (strlen(branch) != 40)
return branch;
- sprintf(githead_env, "GITHEAD_%s", branch);
+ xsnprintf(githead_env, sizeof(githead_env), "GITHEAD_%s", branch);
name = getenv(githead_env);
return name ? name : branch;
}
diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index 2379e1106..8c693e756 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -90,7 +90,7 @@ static int debug_merge(const struct cache_entry * const *stages,
debug_stage("index", stages[0], o);
for (i = 1; i <= o->merge_size; i++) {
char buf[24];
- sprintf(buf, "ent#%d", i);
+ xsnprintf(buf, sizeof(buf), "ent#%d", i);
debug_stage(buf, stages[i], o);
}
return 0;
diff --git a/builtin/unpack-file.c b/builtin/unpack-file.c
index 19200291a..6fc6bcdf7 100644
--- a/builtin/unpack-file.c
+++ b/builtin/unpack-file.c
@@ -12,7 +12,7 @@ static char *create_temp_file(unsigned char *sha1)
if (!buf || type != OBJ_BLOB)
die("unable to read blob object %s", sha1_to_hex(sha1));
- strcpy(path, ".merge_file_XXXXXX");
+ xsnprintf(path, sizeof(path), ".merge_file_XXXXXX");
fd = xmkstemp(path);
if (write_in_full(fd, buf, size) != size)
die_errno("unable to write temp-file");
diff --git a/compat/mingw.c b/compat/mingw.c
index f74da235f..a168800ae 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -2133,9 +2133,11 @@ int uname(struct utsname *buf)
{
DWORD v = GetVersion();
memset(buf, 0, sizeof(*buf));
- strcpy(buf->sysname, "Windows");
- sprintf(buf->release, "%u.%u", v & 0xff, (v >> 8) & 0xff);
+ xsnprintf(buf->sysname, sizeof(buf->sysname), "Windows");
+ xsnprintf(buf->release, sizeof(buf->release),
+ "%u.%u", v & 0xff, (v >> 8) & 0xff);
/* assuming NT variants only.. */
- sprintf(buf->version, "%u", (v >> 16) & 0x7fff);
+ xsnprintf(buf->version, sizeof(buf->version),
+ "%u", (v >> 16) & 0x7fff);
return 0;
}
diff --git a/compat/winansi.c b/compat/winansi.c
index efc5bb3a4..ceff55bd6 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -539,7 +539,7 @@ void winansi_init(void)
return;
/* create a named pipe to communicate with the console thread */
- sprintf(name, "\\\\.\\pipe\\winansi%lu", GetCurrentProcessId());
+ xsnprintf(name, sizeof(name), "\\\\.\\pipe\\winansi%lu", GetCurrentProcessId());
hwrite = CreateNamedPipe(name, PIPE_ACCESS_OUTBOUND,
PIPE_TYPE_BYTE | PIPE_WAIT, 1, BUFFER_SIZE, 0, 0, NULL);
if (hwrite == INVALID_HANDLE_VALUE)
diff --git a/connect.c b/connect.c
index c0144d859..1d5c5e005 100644
--- a/connect.c
+++ b/connect.c
@@ -332,7 +332,7 @@ static const char *ai_name(const struct addrinfo *ai)
static char addr[NI_MAXHOST];
if (getnameinfo(ai->ai_addr, ai->ai_addrlen, addr, sizeof(addr), NULL, 0,
NI_NUMERICHOST) != 0)
- strcpy(addr, "(unknown)");
+ xsnprintf(addr, sizeof(addr), "(unknown)");
return addr;
}
diff --git a/convert.c b/convert.c
index f3bd3e93f..814e81443 100644
--- a/convert.c
+++ b/convert.c
@@ -1289,7 +1289,8 @@ static struct stream_filter *ident_filter(const unsigned char *sha1)
{
struct ident_filter *ident = xmalloc(sizeof(*ident));
- sprintf(ident->ident, ": %s $", sha1_to_hex(sha1));
+ xsnprintf(ident->ident, sizeof(ident->ident),
+ ": %s $", sha1_to_hex(sha1));
strbuf_init(&ident->left, 0);
ident->filter.vtbl = &ident_vtbl;
ident->state = 0;
diff --git a/daemon.c b/daemon.c
index f9eb29688..5218a3f1c 100644
--- a/daemon.c
+++ b/daemon.c
@@ -901,7 +901,7 @@ static const char *ip2str(int family, struct sockaddr *sin, socklen_t len)
inet_ntop(family, &((struct sockaddr_in*)sin)->sin_addr, ip, len);
break;
default:
- strcpy(ip, "<unknown>");
+ xsnprintf(ip, sizeof(ip), "<unknown>");
}
return ip;
}
@@ -916,7 +916,7 @@ static int setup_named_sock(char *listen_addr, int listen_port, struct socketlis
int gai;
long flags;
- sprintf(pbuf, "%d", listen_port);
+ xsnprintf(pbuf, sizeof(pbuf), "%d", listen_port);
memset(&hints, 0, sizeof(hints));
hints.ai_family = AF_UNSPEC;
hints.ai_socktype = SOCK_STREAM;
diff --git a/diff.c b/diff.c
index 08508f6a2..788e371de 100644
--- a/diff.c
+++ b/diff.c
@@ -2880,7 +2880,7 @@ static void prep_temp_blob(const char *path, struct diff_tempfile *temp,
temp->name = get_tempfile_path(&temp->tempfile);
strcpy(temp->hex, sha1_to_hex(sha1));
temp->hex[40] = 0;
- sprintf(temp->mode, "%06o", mode);
+ xsnprintf(temp->mode, sizeof(temp->mode), "%06o", mode);
strbuf_release(&buf);
strbuf_release(&template);
free(path_dup);
@@ -2897,8 +2897,8 @@ static struct diff_tempfile *prepare_temp_file(const char *name,
* a '+' entry produces this for file-1.
*/
temp->name = "/dev/null";
- strcpy(temp->hex, ".");
- strcpy(temp->mode, ".");
+ xsnprintf(temp->hex, sizeof(temp->hex), ".");
+ xsnprintf(temp->mode, sizeof(temp->mode), ".");
return temp;
}
@@ -2935,7 +2935,7 @@ static struct diff_tempfile *prepare_temp_file(const char *name,
* !(one->sha1_valid), as long as
* DIFF_FILE_VALID(one).
*/
- sprintf(temp->mode, "%06o", one->mode);
+ xsnprintf(temp->mode, sizeof(temp->mode), "%06o", one->mode);
}
return temp;
}
@@ -4081,9 +4081,9 @@ const char *diff_unique_abbrev(const unsigned char *sha1, int len)
if (abblen < 37) {
static char hex[41];
if (len < abblen && abblen <= len + 2)
- sprintf(hex, "%s%.*s", abbrev, len+3-abblen, "..");
+ xsnprintf(hex, sizeof(hex), "%s%.*s", abbrev, len+3-abblen, "..");
else
- sprintf(hex, "%s...", abbrev);
+ xsnprintf(hex, sizeof(hex), "%s...", abbrev);
return hex;
}
return sha1_to_hex(sha1);
diff --git a/http-push.c b/http-push.c
index c98dad23d..154e67bb0 100644
--- a/http-push.c
+++ b/http-push.c
@@ -881,7 +881,7 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
strbuf_addf(&out_buffer.buf, LOCK_REQUEST, escaped);
free(escaped);
- sprintf(timeout_header, "Timeout: Second-%ld", timeout);
+ xsnprintf(timeout_header, sizeof(timeout_header), "Timeout: Second-%ld", timeout);
dav_headers = curl_slist_append(dav_headers, timeout_header);
dav_headers = curl_slist_append(dav_headers, "Content-Type: text/xml");
diff --git a/http.c b/http.c
index 9dce38025..7b0225996 100644
--- a/http.c
+++ b/http.c
@@ -1104,7 +1104,7 @@ static void write_accept_language(struct strbuf *buf)
decimal_places++, max_q *= 10)
;
- sprintf(q_format, ";q=0.%%0%dd", decimal_places);
+ xsnprintf(q_format, sizeof(q_format), ";q=0.%%0%dd", decimal_places);
strbuf_addstr(buf, "Accept-Language: ");
@@ -1601,7 +1601,7 @@ struct http_pack_request *new_http_pack_request(
fprintf(stderr,
"Resuming fetch of pack %s at byte %ld\n",
sha1_to_hex(target->sha1), prev_posn);
- sprintf(range, "Range: bytes=%ld-", prev_posn);
+ xsnprintf(range, sizeof(range), "Range: bytes=%ld-", prev_posn);
preq->range_header = curl_slist_append(NULL, range);
curl_easy_setopt(preq->slot->curl, CURLOPT_HTTPHEADER,
preq->range_header);
@@ -1761,7 +1761,7 @@ struct http_object_request *new_http_object_request(const char *base_url,
fprintf(stderr,
"Resuming fetch of object %s at byte %ld\n",
hex, prev_posn);
- sprintf(range, "Range: bytes=%ld-", prev_posn);
+ xsnprintf(range, sizeof(range), "Range: bytes=%ld-", prev_posn);
range_header = curl_slist_append(range_header, range);
curl_easy_setopt(freq->slot->curl,
CURLOPT_HTTPHEADER, range_header);
diff --git a/ll-merge.c b/ll-merge.c
index fc3c04959..56f73b393 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -142,11 +142,11 @@ static struct ll_merge_driver ll_merge_drv[] = {
{ "union", "built-in union merge", ll_union_merge },
};
-static void create_temp(mmfile_t *src, char *path)
+static void create_temp(mmfile_t *src, char *path, size_t len)
{
int fd;
- strcpy(path, ".merge_file_XXXXXX");
+ xsnprintf(path, len, ".merge_file_XXXXXX");
fd = xmkstemp(path);
if (write_in_full(fd, src->ptr, src->size) != src->size)
die_errno("unable to write temp-file");
@@ -187,10 +187,10 @@ static int ll_ext_merge(const struct ll_merge_driver *fn,
result->ptr = NULL;
result->size = 0;
- create_temp(orig, temp[0]);
- create_temp(src1, temp[1]);
- create_temp(src2, temp[2]);
- sprintf(temp[3], "%d", marker_size);
+ create_temp(orig, temp[0], sizeof(temp[0]));
+ create_temp(src1, temp[1], sizeof(temp[1]));
+ create_temp(src2, temp[2], sizeof(temp[2]));
+ xsnprintf(temp[3], sizeof(temp[3]), "%d", marker_size);
strbuf_expand(&cmd, fn->cmdline, strbuf_expand_dict_cb, &dict);
diff --git a/refs.c b/refs.c
index 4e15f60d9..d5c8b2f51 100644
--- a/refs.c
+++ b/refs.c
@@ -3326,10 +3326,10 @@ static int log_ref_write_fd(int fd, const unsigned char *old_sha1,
msglen = msg ? strlen(msg) : 0;
maxlen = strlen(committer) + msglen + 100;
logrec = xmalloc(maxlen);
- len = sprintf(logrec, "%s %s %s\n",
- sha1_to_hex(old_sha1),
- sha1_to_hex(new_sha1),
- committer);
+ len = xsnprintf(logrec, maxlen, "%s %s %s\n",
+ sha1_to_hex(old_sha1),
+ sha1_to_hex(new_sha1),
+ committer);
if (msglen)
len += copy_msg(logrec + len - 1, msg) - 1;
diff --git a/sideband.c b/sideband.c
index 7f9dc229f..fde8adc00 100644
--- a/sideband.c
+++ b/sideband.c
@@ -137,11 +137,11 @@ ssize_t send_sideband(int fd, int band, const char *data, ssize_t sz, int packet
if (packet_max - 5 < n)
n = packet_max - 5;
if (0 <= band) {
- sprintf(hdr, "%04x", n + 5);
+ xsnprintf(hdr, sizeof(hdr), "%04x", n + 5);
hdr[4] = band;
write_or_die(fd, hdr, 5);
} else {
- sprintf(hdr, "%04x", n + 4);
+ xsnprintf(hdr, sizeof(hdr), "%04x", n + 4);
write_or_die(fd, hdr, 4);
}
write_or_die(fd, p, n);
diff --git a/strbuf.c b/strbuf.c
index f3c44fb3b..107c45d29 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -245,8 +245,8 @@ void strbuf_add_commented_lines(struct strbuf *out, const char *buf, size_t size
static char prefix2[2];
if (prefix1[0] != comment_line_char) {
- sprintf(prefix1, "%c ", comment_line_char);
- sprintf(prefix2, "%c", comment_line_char);
+ xsnprintf(prefix1, sizeof(prefix1), "%c ", comment_line_char);
+ xsnprintf(prefix2, sizeof(prefix2), "%c", comment_line_char);
}
add_lines(out, prefix1, prefix2, buf, size);
}