From db85a8a9c2fb492d3cd528dbbcc52075c607cf79 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 24 Sep 2015 17:06:06 -0400 Subject: compat/inet_ntop: fix off-by-one in inet_ntop4 Our compat inet_ntop4 function writes to a temporary buffer with snprintf, and then uses strcpy to put the result into the final "dst" buffer. We check the return value of snprintf against the size of "dst", but fail to account for the NUL terminator. As a result, we may overflow "dst" with a single NUL. In practice, this doesn't happen because the output of inet_ntop is limited, and we provide buffers that are way oversized. We can fix the off-by-one check easily, but while we are here let's also use strlcpy for increased safety, just in case there are other bugs lurking. As a side note, this compat code seems to be BSD-derived. Searching for "vixie inet_ntop" turns up NetBSD's latest version of the same code, which has an identical fix (and switches to strlcpy, too!). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- compat/inet_ntop.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'compat') diff --git a/compat/inet_ntop.c b/compat/inet_ntop.c index 90b7cc45f3..68307262be 100644 --- a/compat/inet_ntop.c +++ b/compat/inet_ntop.c @@ -53,11 +53,11 @@ inet_ntop4(const u_char *src, char *dst, size_t size) nprinted = snprintf(tmp, sizeof(tmp), fmt, src[0], src[1], src[2], src[3]); if (nprinted < 0) return (NULL); /* we assume "errno" was set by "snprintf()" */ - if ((size_t)nprinted > size) { + if ((size_t)nprinted >= size) { errno = ENOSPC; return (NULL); } - strcpy(dst, tmp); + strlcpy(dst, tmp, size); return (dst); } @@ -154,7 +154,7 @@ inet_ntop6(const u_char *src, char *dst, size_t size) errno = ENOSPC; return (NULL); } - strcpy(dst, tmp); + strlcpy(dst, tmp, size); return (dst); } #endif -- cgit v1.3-5-g9baa From 5096d4909f9b13c7a650d9dbb7c9702ea7413566 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 24 Sep 2015 17:06:08 -0400 Subject: 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 Signed-off-by: Junio C Hamano --- archive-tar.c | 2 +- builtin/gc.c | 2 +- builtin/init-db.c | 11 ++++++----- builtin/ls-tree.c | 9 +++++---- builtin/merge-index.c | 2 +- builtin/merge-recursive.c | 2 +- builtin/read-tree.c | 2 +- builtin/unpack-file.c | 2 +- compat/mingw.c | 8 +++++--- compat/winansi.c | 2 +- connect.c | 2 +- convert.c | 3 ++- daemon.c | 4 ++-- diff.c | 12 ++++++------ http-push.c | 2 +- http.c | 6 +++--- ll-merge.c | 12 ++++++------ refs.c | 8 ++++---- sideband.c | 4 ++-- strbuf.c | 4 ++-- 20 files changed, 52 insertions(+), 47 deletions(-) (limited to 'compat') diff --git a/archive-tar.c b/archive-tar.c index b6b30bb577..d543f93fc9 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 0ad8d30b56..57584bc99f 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 69323e186c..e7d0e31f46 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 3b04a0f082..0e30d86230 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 1a1eafa6fd..1d66111917 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 a90f28f34d..491efd556e 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 2379e11069..8c693e7568 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 19200291a2..6fc6bcdf7f 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 f74da235f5..a168800ae0 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 efc5bb3a4b..ceff55bd67 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 c0144d859a..1d5c5e0055 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 f3bd3e93fb..814e814438 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 f9eb296888..5218a3f1cc 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, ""); + xsnprintf(ip, sizeof(ip), ""); } 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 08508f6a20..788e371dec 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 c98dad23df..154e67bb05 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 9dce38025c..7b02259961 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 fc3c049594..56f73b3932 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 4e15f60d98..d5c8b2f51b 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 7f9dc229fb..fde8adc000 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 f3c44fb3b3..107c45d291 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); } -- cgit v1.3-5-g9baa From 48bdf86995423736f36557d744841b08c8bf4e14 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 24 Sep 2015 17:06:48 -0400 Subject: compat/hstrerror: convert sprintf to snprintf This is a trivially correct use of sprintf, as our error number should not be excessively long. But it's still nice to drop an sprintf call. Note that we cannot use xsnprintf here, because this is compat code which does not load git-compat-util.h. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- compat/hstrerror.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'compat') diff --git a/compat/hstrerror.c b/compat/hstrerror.c index 069c555da4..b85a2fa956 100644 --- a/compat/hstrerror.c +++ b/compat/hstrerror.c @@ -16,6 +16,6 @@ const char *githstrerror(int err) case TRY_AGAIN: return "Non-authoritative \"host not found\", or SERVERFAIL"; } - sprintf(buffer, "Name resolution error %d", err); + snprintf(buffer, sizeof(buffer), "Name resolution error %d", err); return buffer; } -- cgit v1.3-5-g9baa From e2b021eb5b5ce2f4be3c10f1f8063981b1a53053 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sun, 4 Oct 2015 23:43:14 -0400 Subject: precompose_utf8: drop unused variable The result of iconv is assigned to a variable, but we never use it (instead, we check errno and whether the function consumed all bytes). Let's drop the assignment, as it triggers gcc's -Wunused-but-set-variable. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- compat/precompose_utf8.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'compat') diff --git a/compat/precompose_utf8.c b/compat/precompose_utf8.c index 95fe849e42..044c68617b 100644 --- a/compat/precompose_utf8.c +++ b/compat/precompose_utf8.c @@ -139,9 +139,8 @@ struct dirent_prec_psx *precompose_utf8_readdir(PREC_DIR *prec_dir) size_t inleft = namelenz; char *outpos = &prec_dir->dirent_nfc->d_name[0]; size_t outsz = prec_dir->dirent_nfc->max_name_len; - size_t cnt; errno = 0; - cnt = iconv(prec_dir->ic_precompose, &cp, &inleft, &outpos, &outsz); + iconv(prec_dir->ic_precompose, &cp, &inleft, &outpos, &outsz); if (errno || inleft) { /* * iconv() failed and errno could be E2BIG, EILSEQ, EINVAL, EBADF -- cgit v1.3-5-g9baa From fdf729661a777d8bd598f40055d92b2df5601332 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sun, 4 Oct 2015 23:45:26 -0400 Subject: probe_utf8_pathname_composition: use internal strbuf When we are initializing a .git directory, we may call probe_utf8_pathname_composition to detect utf8 mangling. We pass in a path buffer for it to use, and it blindly strcpy()s into it, not knowing whether the buffer is large enough to hold the result or not. In practice this isn't a big deal, because the buffer we pass in already contains "$GIT_DIR/config", and we append only a few extra bytes to it. But we can easily do the right thing just by calling git_path_buf ourselves. Technically this results in a different pathname (before we appended our utf8 characters to the "config" path, and now they get their own files in $GIT_DIR), but that should not matter for our purposes. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/init-db.c | 2 +- compat/precompose_utf8.c | 18 ++++++++++-------- compat/precompose_utf8.h | 2 +- git-compat-util.h | 2 +- 4 files changed, 13 insertions(+), 11 deletions(-) (limited to 'compat') diff --git a/builtin/init-db.c b/builtin/init-db.c index e7d0e31f46..89addda4fd 100644 --- a/builtin/init-db.c +++ b/builtin/init-db.c @@ -312,7 +312,7 @@ static int create_default_files(const char *template_path) strcpy(path + len, "CoNfIg"); if (!access(path, F_OK)) git_config_set("core.ignorecase", "true"); - probe_utf8_pathname_composition(path, len); + probe_utf8_pathname_composition(); } return reinit; diff --git a/compat/precompose_utf8.c b/compat/precompose_utf8.c index 044c68617b..079070ff1d 100644 --- a/compat/precompose_utf8.c +++ b/compat/precompose_utf8.c @@ -36,24 +36,26 @@ static size_t has_non_ascii(const char *s, size_t maxlen, size_t *strlen_c) } -void probe_utf8_pathname_composition(char *path, int len) +void probe_utf8_pathname_composition(void) { + struct strbuf path = STRBUF_INIT; static const char *auml_nfc = "\xc3\xa4"; static const char *auml_nfd = "\x61\xcc\x88"; int output_fd; if (precomposed_unicode != -1) return; /* We found it defined in the global config, respect it */ - strcpy(path + len, auml_nfc); - output_fd = open(path, O_CREAT|O_EXCL|O_RDWR, 0600); + git_path_buf(&path, "%s", auml_nfc); + output_fd = open(path.buf, O_CREAT|O_EXCL|O_RDWR, 0600); if (output_fd >= 0) { close(output_fd); - strcpy(path + len, auml_nfd); - precomposed_unicode = access(path, R_OK) ? 0 : 1; + git_path_buf(&path, "%s", auml_nfd); + precomposed_unicode = access(path.buf, R_OK) ? 0 : 1; git_config_set("core.precomposeunicode", precomposed_unicode ? "true" : "false"); - strcpy(path + len, auml_nfc); - if (unlink(path)) - die_errno(_("failed to unlink '%s'"), path); + git_path_buf(&path, "%s", auml_nfc); + if (unlink(path.buf)) + die_errno(_("failed to unlink '%s'"), path.buf); } + strbuf_release(&path); } diff --git a/compat/precompose_utf8.h b/compat/precompose_utf8.h index 3b73585fc5..a94e7c4342 100644 --- a/compat/precompose_utf8.h +++ b/compat/precompose_utf8.h @@ -27,7 +27,7 @@ typedef struct { } PREC_DIR; void precompose_argv(int argc, const char **argv); -void probe_utf8_pathname_composition(char *, int); +void probe_utf8_pathname_composition(void); PREC_DIR *precompose_utf8_opendir(const char *dirname); struct dirent_prec_psx *precompose_utf8_readdir(PREC_DIR *dirp); diff --git a/git-compat-util.h b/git-compat-util.h index 348b9dcc1c..9a3e5591cb 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -229,7 +229,7 @@ typedef unsigned long uintptr_t; #else #define precompose_str(in,i_nfd2nfc) #define precompose_argv(c,v) -#define probe_utf8_pathname_composition(a,b) +#define probe_utf8_pathname_composition() #endif #ifdef MKDIR_WO_TRAILING_SLASH -- cgit v1.3-5-g9baa From 34fa79a6cde56d6d428ab0d3160cb094ebad3305 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 24 Sep 2015 17:08:19 -0400 Subject: prefer memcpy to strcpy When we already know the length of a string (e.g., because we just malloc'd to fit it), it's nicer to use memcpy than strcpy, as it makes it more obvious that we are not going to overflow the buffer (because the size we pass matches the size in the allocation). This also eliminates calls to strcpy, which make auditing the code base harder. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- compat/nedmalloc/nedmalloc.c | 5 +++-- fast-import.c | 5 +++-- revision.c | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) (limited to 'compat') diff --git a/compat/nedmalloc/nedmalloc.c b/compat/nedmalloc/nedmalloc.c index 609ebba125..a0a16eb1bb 100644 --- a/compat/nedmalloc/nedmalloc.c +++ b/compat/nedmalloc/nedmalloc.c @@ -957,8 +957,9 @@ char *strdup(const char *s1) { char *s2 = 0; if (s1) { - s2 = malloc(strlen(s1) + 1); - strcpy(s2, s1); + size_t len = strlen(s1) + 1; + s2 = malloc(len); + memcpy(s2, s1, len); } return s2; } diff --git a/fast-import.c b/fast-import.c index 895c6b4a7e..cf6d8bc0ce 100644 --- a/fast-import.c +++ b/fast-import.c @@ -644,8 +644,9 @@ static void *pool_calloc(size_t count, size_t size) static char *pool_strdup(const char *s) { - char *r = pool_alloc(strlen(s) + 1); - strcpy(r, s); + size_t len = strlen(s) + 1; + char *r = pool_alloc(len); + memcpy(r, s, len); return r; } diff --git a/revision.c b/revision.c index af2a18ed74..22364636d1 100644 --- a/revision.c +++ b/revision.c @@ -38,7 +38,7 @@ char *path_name(const struct name_path *path, const char *name) } n = xmalloc(len); m = n + len - (nlen + 1); - strcpy(m, name); + memcpy(m, name, nlen + 1); for (p = path; p; p = p->up) { if (p->elem_len) { m -= p->elem_len + 1; -- cgit v1.3-5-g9baa