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>

diff --git a/archive-tar.c b/archive-tar.c
index b6b30bb577679f3b012cd50038dbed52fb6cd81e..d543f93fc9f70ee2884ba284fd02515473a864a4 100644 (file)
--- 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 0ad8d30b56f89a9ea6ac3a9ee5ae4593ae9754a0..57584bc99faf49afe96caa16cee3c04f5ad64be5 100644 (file)
--- 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 69323e186cda73fe02658a535d8eae8387ff5444..e7d0e31f46e9890a267fa508fa43d184d563afed 100644 (file)
--- 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 3b04a0f082a48c7b9c16f57a5bf4be94c159204c..0e30d862303b67ace2c7accf2e7c9346d8f63264 100644 (file)
--- 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 1a1eafa6fdc2e9dc66c98d3a81b7ee50e44727da..1d6611191791ff66822b4a534e3c8dd1a6b9e473 100644 (file)
--- 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 a90f28f34d2072b7d40964a7fa3cf46d3bb98cdd..491efd556e87d0b263bb1029e0cabb9b7f6f1472 100644 (file)
--- 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 2379e11069ef223a82580a9451ede8fbdb1eb8d9..8c693e756852ef2ec3aac3bfc77d71c6258a8306 100644 (file)
--- 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 19200291a2632554223b1a675d5324b4682248ab..6fc6bcdf7f014a52703fcd44677a705c824bc0b0 100644 (file)
--- 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 f74da235f598d8b0346fac8b48c4155f55ae5b81..a168800ae0d1a6ffe741dfda26dbe5133cc4c4fc 100644 (file)
--- 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 efc5bb3a4b63166eccb33f6ec6a8ad57e4c9ac36..ceff55bd67696403f374f68ae6d3db97f603aebe 100644 (file)
--- 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 c0144d859ae4275860df464f73a688c649d092fe..1d5c5e005557ae63e31685b1d4fc8c3a3a0b9ada 100644 (file)
--- 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 f3bd3e93fb2ecf95413db3c53d7e686cd03d1e69..814e814438b7c0f4f84850787670766fb4765f08 100644 (file)
--- 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 f9eb296888c37c8f2a523f3941be027ddb36df3c..5218a3f1ccc532f4774fbcc7433daea39dade371 100644 (file)
--- 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 08508f6a2017eb35a350268ea78a44cfc1d867e4..788e371decee39fb2aa0048c8b93f322d7ffa832 100644 (file)
--- 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 c98dad23dfd86875209295fadda39c1f1b4f5224..154e67bb05f0ccec2fdd42769b352f78d9f937c8 100644 (file)
--- 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 9dce38025c7b35f7f5ad19121c0529b442aec92d..7b0225996189fc5e860249b484947285a1281d63 100644 (file)
--- 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 fc3c0495942e2a8d7c31a087429f894633c603d8..56f73b393256091194d88eaf9d1b87776dc06dff 100644 (file)
--- 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 4e15f60d98ea8affdef226bce199935fa694b195..d5c8b2f51b8ad6e1c035db12d809d5ca9111d381 100644 (file)
--- 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 7f9dc229fbc82d005b5c2b417c2af2fc9f6b497a..fde8adc000f3167bba7fdf2d7cb3a4e7db612af4 100644 (file)
--- 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 f3c44fb3b33b1419c2d2952a74477467fa8f6c4d..107c45d29111a011a04daaa8fd0323ed9d973398 100644 (file)
--- 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);
 }