Fixed issue #1743: Changed sprintf to safer snprintf

Also changed a few vsprintf to vsnprintf.

Most cases were trivial, one private function was changed to take the buffer length, one case required some fancy arithmetic.

2 files changed, 21 insertions, 20 deletions

diff --git a/tar/test/test_copy.c b/tar/test/test_copy.c
index d618e45c..b8175c35 100644
--- a/tar/test/test_copy.c
+++ b/tar/test/test_copy.c
@@ -160,21 +160,21 @@ create_tree(void)
 		failure("Internal sanity check failed: i = %d", i);
 		assert(filenames[i] != NULL);
 
-		sprintf(buff, "f/%s", filenames[i]);
+		snprintf(buff, sizeof(buff), "f/%s", filenames[i]);
 		assertMakeFile(buff, 0777, buff);
 
 		/* Create a link named "l/abcdef..." to the above. */
-		sprintf(buff2, "l/%s", filenames[i]);
+		snprintf(buff2, sizeof(buff2), "l/%s", filenames[i]);
 		assertMakeHardlink(buff2, buff);
 
 		/* Create a link named "m/abcdef..." to the above. */
-		sprintf(buff2, "m/%s", filenames[i]);
+		snprintf(buff2, sizeof(buff2), "m/%s", filenames[i]);
 		assertMakeHardlink(buff2, buff);
 
 		if (canSymlink()) {
 			/* Create a symlink named "s/abcdef..." to the above. */
-			sprintf(buff, "s/%s", filenames[i]);
-			sprintf(buff2, "../f/%s", filenames[i]);
+			snprintf(buff, sizeof(buff), "s/%s", filenames[i]);
+			snprintf(buff2, sizeof(buff2), "../f/%s", filenames[i]);
 			failure("buff=\"%s\" buff2=\"%s\"", buff, buff2);
 			assertMakeSymlink(buff, buff2, 0);
 		}
@@ -202,13 +202,13 @@ verify_tree(size_t limit)
 	/* Generate the names we know should be there and verify them. */
 	for (i = 1; i < LOOP_MAX; i++) {
 		/* Verify a file named "f/abcdef..." */
-		sprintf(name1, "f/%s", filenames[i]);
+		snprintf(name1, sizeof(name1), "f/%s", filenames[i]);
 		if (i <= limit) {
 			assertFileExists(name1);
 			assertFileContents(name1, (int)strlen(name1), name1);
 		}
 
-		sprintf(name2, "l/%s", filenames[i]);
+		snprintf(name2, sizeof(name2), "l/%s", filenames[i]);
 		if (i + 2 <= limit) {
 			/* Verify hardlink "l/abcdef..." */
 			assertIsHardlink(name1, name2);
@@ -219,14 +219,14 @@ verify_tree(size_t limit)
 
 		if (canSymlink()) {
 			/* Verify symlink "s/abcdef..." */
-			sprintf(name1, "s/%s", filenames[i]);
-			sprintf(name2, "../f/%s", filenames[i]);
+			snprintf(name1, sizeof(name1), "s/%s", filenames[i]);
+			snprintf(name2, sizeof(name2), "../f/%s", filenames[i]);
 			if (strlen(name2) <= limit)
 				assertIsSymlink(name1, name2, 0);
 		}
 
 		/* Verify dir "d/abcdef...". */
-		sprintf(name1, "d/%s", filenames[i]);
+		snprintf(name1, sizeof(name1), "d/%s", filenames[i]);
 		if (i + 1 <= limit) { /* +1 for trailing slash */
 			if (assertIsDir(name1, -1)) {
 				/* TODO: opendir/readdir this
diff --git a/tar/util.c b/tar/util.c
index 8ebec64c..5a4ab0b3 100644
--- a/tar/util.c
+++ b/tar/util.c
@@ -63,7 +63,7 @@ __FBSDID("$FreeBSD: src/usr.bin/tar/util.c,v 1.23 2008/12/15 06:00:25 kientzle E
 #include "err.h"
 #include "passphrase.h"
 
-static size_t	bsdtar_expand_char(char *, size_t, char);
+static size_t	bsdtar_expand_char(char *, size_t, size_t, char);
 static const char *strip_components(const char *path, int elements);
 
 #if defined(_WIN32) && !defined(__CYGWIN__)
@@ -173,12 +173,12 @@ safe_fprintf(FILE *f, const char *fmt, ...)
 				/* Not printable, format the bytes. */
 				while (n-- > 0)
 					i += (unsigned)bsdtar_expand_char(
-					    outbuff, i, *p++);
+					    outbuff, sizeof(outbuff), i, *p++);
 			}
 		} else {
 			/* After any conversion failure, don't bother
 			 * trying to convert the rest. */
-			i += (unsigned)bsdtar_expand_char(outbuff, i, *p++);
+			i += (unsigned)bsdtar_expand_char(outbuff, sizeof(outbuff), i, *p++);
 			try_wc = 0;
 		}
 
@@ -200,7 +200,7 @@ safe_fprintf(FILE *f, const char *fmt, ...)
  * Render an arbitrary sequence of bytes into printable ASCII characters.
  */
 static size_t
-bsdtar_expand_char(char *buff, size_t offset, char c)
+bsdtar_expand_char(char *buff, size_t buffsize, size_t offset, char c)
 {
 	size_t i = offset;
 
@@ -221,7 +221,7 @@ bsdtar_expand_char(char *buff, size_t offset, char c)
 		case '\v': buff[i++] = 'v'; break;
 		case '\\': buff[i++] = '\\'; break;
 		default:
-			sprintf(buff + i, "%03o", 0xFF & (int)c);
+			snprintf(buff + i, buffsize - i, "%03o", 0xFF & (int)c);
 			i += 3;
 		}
 	}
@@ -309,11 +309,12 @@ set_chdir(struct bsdtar *bsdtar, const char *newdir)
 		/* The -C /foo -C bar case; concatenate */
 		char *old_pending = bsdtar->pending_chdir;
 		size_t old_len = strlen(old_pending);
-		bsdtar->pending_chdir = malloc(old_len + strlen(newdir) + 2);
+        size_t new_len = old_len + strlen(newdir) + 2;
+		bsdtar->pending_chdir = malloc(new_len);
 		if (old_pending[old_len - 1] == '/')
 			old_pending[old_len - 1] = '\0';
 		if (bsdtar->pending_chdir != NULL)
-			sprintf(bsdtar->pending_chdir, "%s/%s",
+			snprintf(bsdtar->pending_chdir, new_len, "%s/%s",
 			    old_pending, newdir);
 		free(old_pending);
 	}
@@ -695,7 +696,7 @@ list_item_verbose(struct bsdtar *bsdtar, FILE *out, struct archive_entry *entry)
 	/* Use uname if it's present, else uid. */
 	p = archive_entry_uname(entry);
 	if ((p == NULL) || (*p == '\0')) {
-		sprintf(tmp, "%lu ",
+		snprintf(tmp, sizeof(tmp), "%lu ",
 		    (unsigned long)archive_entry_uid(entry));
 		p = tmp;
 	}
@@ -710,7 +711,7 @@ list_item_verbose(struct bsdtar *bsdtar, FILE *out, struct archive_entry *entry)
 		fprintf(out, "%s", p);
 		w = strlen(p);
 	} else {
-		sprintf(tmp, "%lu",
+		snprintf(tmp, sizeof(tmp), "%lu",
 		    (unsigned long)archive_entry_gid(entry));
 		w = strlen(tmp);
 		fprintf(out, "%s", tmp);
@@ -723,7 +724,7 @@ list_item_verbose(struct bsdtar *bsdtar, FILE *out, struct archive_entry *entry)
 	 */
 	if (archive_entry_filetype(entry) == AE_IFCHR
 	    || archive_entry_filetype(entry) == AE_IFBLK) {
-		sprintf(tmp, "%lu,%lu",
+		snprintf(tmp, sizeof(tmp), "%lu,%lu",
 		    (unsigned long)archive_entry_rdevmajor(entry),
 		    (unsigned long)archive_entry_rdevminor(entry));
 	} else {