tech-userlevel archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: bin/pax: Adding support for base-256 (GNU-style) encoded file sizes



On Wed, 2018-11-28 at 08:20 +0700, Robert Elz wrote:
>     Date:        Wed, 28 Nov 2018 00:23:47 +0100
>     From:        =?UTF-8?Q?Micha=C5=82_G=C3=B3rny?= <mgorny%gentoo.org@localhost>
>     Message-ID:  <1543361027.17222.11.camel%gentoo.org@localhost>
> 
> I have no idea whether this is something that we want or not,
> but assuming that we do ...
> 
>   | so I'd appreciate some pointers if I'm doing things right.
> 
> I think you need to watch for sign extension from *str (str is a char *, not 
> u_char *) which probably means &0xFF is needed, and I'd suggest
> that rather than doing <<8 >> 8 and comparing for equality, testing
> whether you're going to overflow when doing a <<8 is better done as
> 	if (tval > (UINTMAX_MAX/256))
> 
> I also see that the other conversions work if len == 0 (they return 0),
> which your one doesn't, it might be worth a "len > 0" in the initial test
> (so you never look at *str if len == 0).

Thank you for your comments.  FTR, I don't think those functions can
even take len==0 (given they're passed only positive values by design)
but I suppose there's no harm in checking for it explicitly.

I've also changed the test case to check whether *str >= 0x80 works
correctly.

Here's the updated patch:

Index: bin/pax/gen_subs.c
===================================================================
RCS file: /pub/NetBSD-CVS/src/bin/pax/gen_subs.c,v
retrieving revision 1.36
diff -u -B -d -u -p -r1.36 gen_subs.c
--- bin/pax/gen_subs.c	9 Aug 2012 08:09:21 -0000	1.36
+++ bin/pax/gen_subs.c	28 Nov 2018 17:23:45 -0000
@@ -306,12 +306,10 @@ u32_asc(uintmax_t val, char *str, int le
 
 /*
  * asc_umax()
- *	convert hex/octal character string into a uintmax. We do
- *	not have to to check for overflow! (the headers in all
supported
- *	formats are not large enough to create an overflow).
+ *	convert hex/octal/base-256 value into a uintmax.
  *	NOTE: strings passed to us are NOT TERMINATED.
  * Return:
- *	uintmax_t value
+ *	uintmax_t value; UINTMAX_MAX for overflow/negative
  */
 
 uintmax_t
@@ -323,6 +321,30 @@ asc_umax(char *str, int len, int base)
 	stop = str + len;
 
 	/*
+	 * if the highest bit of first byte is set, it's base-256 encoded
+	 * (base-256 is basically (n-1)-bit big endian signed
+	 */
+	if (str < stop && (*str & 0x80)) {
+		/*
+		 * uintmax_t can't be negative, so fail on negative numbers
+		 */
+		if (*str & 0x40)
+			return UINTMAX_MAX;
+
+		tval = *str++ & 0x3f;
+		while (str < stop) {
+			/*
+			 * check for overflow
+			 */
+			if (tval > (UINTMAX_MAX/256))
+				return UINTMAX_MAX;
+			tval = (tval << 8) | ((*str++) & 0xFF);
+		}
+
+		return tval;
+	}
+
+	/*
 	 * skip over leading blanks and zeros
 	 */
 	while ((str < stop) && ((*str == ' ') || (*str == '0')))
Index: bin/pax/tar.c
===================================================================
RCS file: /pub/NetBSD-CVS/src/bin/pax/tar.c,v
retrieving revision 1.73
diff -u -B -d -u -p -r1.73 tar.c
--- bin/pax/tar.c	19 Dec 2015 18:28:54 -0000	1.73
+++ bin/pax/tar.c	28 Nov 2018 17:23:45 -0000
@@ -486,6 +486,8 @@ tar_rd(ARCHD *arcn, char *buf)
 	arcn->sb.st_uid = (uid_t)asc_u32(hd->uid, sizeof(hd->uid),
OCT);
 	arcn->sb.st_gid = (gid_t)asc_u32(hd->gid, sizeof(hd->gid), OCT);
 	arcn->sb.st_size = (off_t)ASC_OFFT(hd->size, sizeof(hd->size), OCT);
+	if (arcn->sb.st_size == -1)
+		return -1;
 	arcn->sb.st_mtime = (time_t)(int32_t)asc_u32(hd->mtime, sizeof(hd->mtime), OCT);
 	arcn->sb.st_ctime = arcn->sb.st_atime = arcn->sb.st_mtime;
 
@@ -860,6 +862,8 @@ ustar_rd(ARCHD *arcn, char *buf)
 	arcn->sb.st_mode = (mode_t)(asc_u32(hd->mode, sizeof(hd->mode), OCT) &
 	    0xfff);
 	arcn->sb.st_size = (off_t)ASC_OFFT(hd->size, sizeof(hd->size), OCT);
+	if (arcn->sb.st_size == -1)
+		return -1;
 	arcn->sb.st_mtime = (time_t)(int32_t)asc_u32(hd->mtime, sizeof(hd->mtime), OCT);
 	arcn->sb.st_ctime = arcn->sb.st_atime = arcn->sb.st_mtime;
 
Index: tests/bin/tar/t_tar.sh
===================================================================
RCS file: /pub/NetBSD-CVS/src/tests/bin/tar/t_tar.sh,v
retrieving revision 1.1
diff -u -B -d -u -p -r1.1 t_tar.sh
--- tests/bin/tar/t_tar.sh	17 Mar 2012 16:33:11 -0000	1.1
+++ tests/bin/tar/t_tar.sh	28 Nov 2018 17:23:45 -0000
@@ -45,7 +45,53 @@ append_body() {
 	atf_check -s eq:0 -o empty -e empty cmp file1.tar file2.tar
 }
 
+atf_test_case rd_base256_size
+rd_base256_size_head() {
+	atf_set "descr" "Test extracting an archive whose member size"
\
+	                "is encoded as base-256 number (GNU style)"
+}
+rd_base256_size_body() {
+	# prepare random file data for comparison
+	# take 0x1200CF bytes in order to test that we:
+	# - handle multiple bytes of size field correctly
+	# - do not fail on NUL bytes
+	# - do not fail on char values > 0x80 (with signed char)
+	dd if=/dev/urandom of=reference.bin bs=1179855 count=1
+	# write test archive header
+	# - filename
+	printf 'output.bin' > test.tar
+	# - pad to 100 octets
+	head -c 90 /dev/zero >> test.tar
+	# - mode, uid, gid
+	printf '%07d\0%07d\0%07d\0' 644 177776 177775 >> test.tar
+	# - size (base-256)
+	printf '\x80\0\0\0\0\0\0\0\0\x12\x00\xCF' >> test.tar
+	# - timestamp, checksum
+	printf '%011d\0%06d\0 0' 13377546642 12460 >> test.tar
+	# - pad empty linkname (100 octets)
+	head -c 100 /dev/zero >> test.tar
+	# - magic, user name
+	printf 'ustar  \0nobody' >> test.tar
+	# - pad user name field to 32 bytes
+	head -c 26 /dev/zero >> test.tar
+	# - group name
+	printf 'nogroup' >> test.tar
+	# - pad to full block
+	head -c 208 /dev/zero >> test.tar
+	# append file data to the test archive
+	cat reference.bin >> test.tar
+	# pad to full block + append two terminating null blocks
+	head -c 1450 /dev/zero >> test.tar
+
+	# test extracting the test archive
+	atf_check -s eq:0 -o empty -e empty tar -xf test.tar
+
+	# ensure that output.bin is equal to reference.bin
+	atf_check -s eq:0 -o empty -e empty cmp output.bin
reference.bin
+}
+
 atf_init_test_cases()
 {
 	atf_add_test_case append
+	atf_add_test_case rd_base256_size
 }

-- 
Best regards,
Michał Górny

Attachment: signature.asc
Description: This is a digitally signed message part



Home | Main Index | Thread Index | Old Index