Jeff King <peff@xxxxxxxx> writes:
> We generally use 32-byte buffers to format git's "type size"
> header fields. These should not generally overflow unless
> you can produce some truly gigantic objects (and our types
> come from our internal array of constant strings). But it is
> a good idea to use xsnprintf to make sure this is the case.
>
> Note that we slightly modify the interface to
> write_sha1_file_prepare, which nows uses "hdrlen" as an "in"
> parameter as well as an "out" (on the way in it stores the
> allocated size of the header, and on the way out it returns
> the ultimate size of the header).
;-). I skipped this paragraph and jumped directly into the code,
noticed that you did something funny in write-sha1-file-prepare and
did the right thing in hash-sha1-file to compensate for it, and then
came back here to find it properly described. Now I read the patch
again and I see the other caller is also properly adjusted.
I think this change makes sense. Thanks.
>
> Signed-off-by: Jeff King <peff@xxxxxxxx>
> ---
> builtin/index-pack.c | 2 +-
> bulk-checkin.c | 4 ++--
> fast-import.c | 4 ++--
> http-push.c | 2 +-
> sha1_file.c | 13 +++++++------
> 5 files changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/builtin/index-pack.c b/builtin/index-pack.c
> index 3431de2..1ad1bde 100644
> --- a/builtin/index-pack.c
> +++ b/builtin/index-pack.c
> @@ -441,7 +441,7 @@ static void *unpack_entry_data(unsigned long offset, unsigned long size,
> int hdrlen;
>
> if (!is_delta_type(type)) {
> - hdrlen = sprintf(hdr, "%s %lu", typename(type), size) + 1;
> + hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %lu", typename(type), size) + 1;
> git_SHA1_Init(&c);
> git_SHA1_Update(&c, hdr, hdrlen);
> } else
> diff --git a/bulk-checkin.c b/bulk-checkin.c
> index 7cffc3a..4347f5c 100644
> --- a/bulk-checkin.c
> +++ b/bulk-checkin.c
> @@ -200,8 +200,8 @@ static int deflate_to_pack(struct bulk_checkin_state *state,
> if (seekback == (off_t) -1)
> return error("cannot find the current offset");
>
> - header_len = sprintf((char *)obuf, "%s %" PRIuMAX,
> - typename(type), (uintmax_t)size) + 1;
> + header_len = xsnprintf((char *)obuf, sizeof(obuf), "%s %" PRIuMAX,
> + typename(type), (uintmax_t)size) + 1;
> git_SHA1_Init(&ctx);
> git_SHA1_Update(&ctx, obuf, header_len);
>
> diff --git a/fast-import.c b/fast-import.c
> index 6c7c3c9..d0c2502 100644
> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -1035,8 +1035,8 @@ static int store_object(
> git_SHA_CTX c;
> git_zstream s;
>
> - hdrlen = sprintf((char *)hdr,"%s %lu", typename(type),
> - (unsigned long)dat->len) + 1;
> + hdrlen = xsnprintf((char *)hdr, sizeof(hdr), "%s %lu",
> + typename(type), (unsigned long)dat->len) + 1;
> git_SHA1_Init(&c);
> git_SHA1_Update(&c, hdr, hdrlen);
> git_SHA1_Update(&c, dat->buf, dat->len);
> diff --git a/http-push.c b/http-push.c
> index 154e67b..1f3788f 100644
> --- a/http-push.c
> +++ b/http-push.c
> @@ -361,7 +361,7 @@ static void start_put(struct transfer_request *request)
> git_zstream stream;
>
> unpacked = read_sha1_file(request->obj->sha1, &type, &len);
> - hdrlen = sprintf(hdr, "%s %lu", typename(type), len) + 1;
> + hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %lu", typename(type), len) + 1;
>
> /* Set it up */
> git_deflate_init(&stream, zlib_compression_level);
> diff --git a/sha1_file.c b/sha1_file.c
> index d295a32..f106091 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -1464,7 +1464,7 @@ int check_sha1_signature(const unsigned char *sha1, void *map,
> return -1;
>
> /* Generate the header */
> - hdrlen = sprintf(hdr, "%s %lu", typename(obj_type), size) + 1;
> + hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %lu", typename(obj_type), size) + 1;
>
> /* Sha1.. */
> git_SHA1_Init(&c);
> @@ -2930,7 +2930,7 @@ static void write_sha1_file_prepare(const void *buf, unsigned long len,
> git_SHA_CTX c;
>
> /* Generate the header */
> - *hdrlen = sprintf(hdr, "%s %lu", type, len)+1;
> + *hdrlen = xsnprintf(hdr, *hdrlen, "%s %lu", type, len)+1;
>
> /* Sha1.. */
> git_SHA1_Init(&c);
> @@ -2993,7 +2993,7 @@ int hash_sha1_file(const void *buf, unsigned long len, const char *type,
> unsigned char *sha1)
> {
> char hdr[32];
> - int hdrlen;
> + int hdrlen = sizeof(hdr);
> write_sha1_file_prepare(buf, len, type, sha1, hdr, &hdrlen);
> return 0;
> }
> @@ -3139,7 +3139,7 @@ static int freshen_packed_object(const unsigned char *sha1)
> int write_sha1_file(const void *buf, unsigned long len, const char *type, unsigned char *sha1)
> {
> char hdr[32];
> - int hdrlen;
> + int hdrlen = sizeof(hdr);
>
> /* Normally if we have it in the pack then we do not bother writing
> * it out into .git/objects/??/?{38} file.
> @@ -3157,7 +3157,8 @@ int hash_sha1_file_literally(const void *buf, unsigned long len, const char *typ
> int hdrlen, status = 0;
>
> /* type string, SP, %lu of the length plus NUL must fit this */
> - header = xmalloc(strlen(type) + 32);
> + hdrlen = strlen(type) + 32;
> + header = xmalloc(hdrlen);
> write_sha1_file_prepare(buf, len, type, sha1, header, &hdrlen);
>
> if (!(flags & HASH_WRITE_OBJECT))
> @@ -3185,7 +3186,7 @@ int force_object_loose(const unsigned char *sha1, time_t mtime)
> buf = read_packed_sha1(sha1, &type, &len);
> if (!buf)
> return error("cannot read sha1_file for %s", sha1_to_hex(sha1));
> - hdrlen = sprintf(hdr, "%s %lu", typename(type), len) + 1;
> + hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %lu", typename(type), len) + 1;
> ret = write_loose_object(sha1, hdr, hdrlen, buf, len, mtime);
> free(buf);
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html