When we generate tar headers, we sprintf() values directly
into a struct with the fixed-size header values. For the
most part this is fine, as we are formatting small values
(e.g., the octal format of "mode & 0x7777" is of fixed
length). But it's still a good idea to use xsnprintf here.
It communicates to readers what our expectation is, and it
provides a run-time check that we are not overflowing the
buffers.

The one exception here is the mtime, which comes from the
epoch time of the commit we are archiving. For sane values,
this fits into the 12-byte value allocated in the header.
But since git can handle 64-bit times, if I claim to be a
visitor from the year 10,000 AD, I can overflow the buffer.
This turns out to be harmless, as we simply overflow into
the chksum field, which is then overwritten.

This case is also best as an xsnprintf. It should never come
up, short of extremely malformed dates, and in that case we
are probably better off dying than silently truncating the
date value (and we cannot expand the size of the buffer,
since it is dictated by the ustar format). Our friends in
the year 5138 (when we legitimately flip to a 12-digit
epoch) can deal with that problem then.

Signed-off-by: Jeff King <p...@peff.net>
---
 archive-tar.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/archive-tar.c b/archive-tar.c
index d543f93..501ca97 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -167,21 +167,21 @@ static void prepare_header(struct archiver_args *args,
                           struct ustar_header *header,
                           unsigned int mode, unsigned long size)
 {
-       sprintf(header->mode, "%07o", mode & 07777);
-       sprintf(header->size, "%011lo", S_ISREG(mode) ? size : 0);
-       sprintf(header->mtime, "%011lo", (unsigned long) args->time);
+       xsnprintf(header->mode, sizeof(header->mode), "%07o", mode & 07777);
+       xsnprintf(header->size, sizeof(header->size), "%011lo", S_ISREG(mode) ? 
size : 0);
+       xsnprintf(header->mtime, sizeof(header->mtime), "%011lo", (unsigned 
long) args->time);
 
-       sprintf(header->uid, "%07o", 0);
-       sprintf(header->gid, "%07o", 0);
+       xsnprintf(header->uid, sizeof(header->uid), "%07o", 0);
+       xsnprintf(header->gid, sizeof(header->gid), "%07o", 0);
        strlcpy(header->uname, "root", sizeof(header->uname));
        strlcpy(header->gname, "root", sizeof(header->gname));
-       sprintf(header->devmajor, "%07o", 0);
-       sprintf(header->devminor, "%07o", 0);
+       xsnprintf(header->devmajor, sizeof(header->devmajor), "%07o", 0);
+       xsnprintf(header->devminor, sizeof(header->devminor), "%07o", 0);
 
        memcpy(header->magic, "ustar", 6);
        memcpy(header->version, "00", 2);
 
-       sprintf(header->chksum, "%07o", ustar_header_chksum(header));
+       snprintf(header->chksum, sizeof(header->chksum), "%07o", 
ustar_header_chksum(header));
 }
 
 static int write_extended_header(struct archiver_args *args,
@@ -193,7 +193,7 @@ static int write_extended_header(struct archiver_args *args,
        memset(&header, 0, sizeof(header));
        *header.typeflag = TYPEFLAG_EXT_HEADER;
        mode = 0100666;
-       sprintf(header.name, "%s.paxheader", sha1_to_hex(sha1));
+       xsnprintf(header.name, sizeof(header.name), "%s.paxheader", 
sha1_to_hex(sha1));
        prepare_header(args, &header, mode, size);
        write_blocked(&header, sizeof(header));
        write_blocked(buffer, size);
@@ -235,8 +235,8 @@ static int write_tar_entry(struct archiver_args *args,
                        memcpy(header.prefix, path, plen);
                        memcpy(header.name, path + plen + 1, rest);
                } else {
-                       sprintf(header.name, "%s.data",
-                               sha1_to_hex(sha1));
+                       xsnprintf(header.name, sizeof(header.name), "%s.data",
+                                 sha1_to_hex(sha1));
                        strbuf_append_ext_header(&ext_header, "path",
                                                 path, pathlen);
                }
@@ -259,8 +259,8 @@ static int write_tar_entry(struct archiver_args *args,
 
        if (S_ISLNK(mode)) {
                if (size > sizeof(header.linkname)) {
-                       sprintf(header.linkname, "see %s.paxheader",
-                               sha1_to_hex(sha1));
+                       xsnprintf(header.linkname, sizeof(header.linkname),
+                                 "see %s.paxheader", sha1_to_hex(sha1));
                        strbuf_append_ext_header(&ext_header, "linkpath",
                                                 buffer, size);
                } else
-- 
2.6.0.rc3.454.g204ad51

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to