[CVE-2021-30139] Out-of-bounds read during tar parsing

Description

apk performs insufficient sanity checks on tar entries. The GNU Tar format specification states the following on fields in tar entries:

The name, linkname, magic, uname, and gname are null-terminated character strings. All other fields are zero-filled octal numbers in ASCII.

The code for parsing tar entries in apk is here:

https://gitlab.alpinelinux.org/alpine/apk-tools/-/blob/354713d2f746c197eed6a1feb4c6af3420af6c15/src/io_archive.c#L143

This code just assumes that uname, etc are null-terminated and uses string function on them without a prior check if null terminators are actually present. For example:

.uid = apk_resolve_uid(idc, buf.uname, GET_OCTAL(buf.uid)),

will use strlen() internally on buf.uname. This will cause an out-of-bounds read. This code is run before the signature is validated.

Reproducing

This issue can be reproduced using out-of-bounds-read-apk-tar-uname.apk and valgrind:

$ valgrind ./src/apk.static add /tmp/out-of-bounds-read-apk-tar-uname.apk
==31584== Memcheck, a memory error detector
==31584== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==31584== Using Valgrind-3.17.0 and LibVEX; rerun with -h for copyright info
==31584== Command: ./src/apk.static --initdb --root ./root/ add /tmp/out-of-bounds-read-apk-tar-uname.apk
==31584== 
==31584== Warning: invalid file descriptor -1 in syscall read()
==31584== Warning: invalid file descriptor -1 in syscall close()
==31584== Warning: invalid file descriptor -1 in syscall read()
==31584== Warning: invalid file descriptor -1 in syscall close()
==31584== Conditional jump or move depends on uninitialised value(s)
==31584==    at 0x5C8CA5: strlen (strlen.c:17)
==31584==    by 0x432575: APK_BLOB_STR (apk_blob.h:79)
==31584==    by 0x4350EB: apk_resolve_uid (io.c:1112)
==31584==    by 0x43696C: apk_tar_parse (io_archive.c:152)
==31584==    by 0x4271BC: apk_pkg_read (package.c:929)
==31584==    by 0x402D75: add_main (app_add.c:163)
==31584==    by 0x40D5FF: main (apk-static.c:516)
==31584== 
==31584== Conditional jump or move depends on uninitialised value(s)
==31584==    at 0x5C8CB0: strlen (strlen.c:20)
==31584==    by 0x432575: APK_BLOB_STR (apk_blob.h:79)
==31584==    by 0x4350EB: apk_resolve_uid (io.c:1112)
==31584==    by 0x43696C: apk_tar_parse (io_archive.c:152)
==31584==    by 0x4271BC: apk_pkg_read (package.c:929)
==31584==    by 0x402D75: add_main (app_add.c:163)
==31584==    by 0x40D5FF: main (apk-static.c:516)
==31584== 
==31584== Use of uninitialised value of size 8
==31584==    at 0x4313FB: apk_hash_get_hashed (hash.c:61)
==31584==    by 0x434F73: resolve_cache_item (io.c:1064)
==31584==    by 0x435107: apk_resolve_uid (io.c:1112)
==31584==    by 0x43696C: apk_tar_parse (io_archive.c:152)
==31584==    by 0x4271BC: apk_pkg_read (package.c:929)
==31584==    by 0x402D75: add_main (app_add.c:163)
==31584==    by 0x40D5FF: main (apk-static.c:516)
==31584== 
==31584== Use of uninitialised value of size 8
==31584==    at 0x4310F0: hlist_add_head (apk_defines.h:231)
==31584==    by 0x4314F4: apk_hash_insert_hashed (hash.c:78)
==31584==    by 0x434FF1: resolve_cache_item (io.c:1074)
==31584==    by 0x435107: apk_resolve_uid (io.c:1112)
==31584==    by 0x43696C: apk_tar_parse (io_archive.c:152)
==31584==    by 0x4271BC: apk_pkg_read (package.c:929)
==31584==    by 0x402D75: add_main (app_add.c:163)
==31584==    by 0x40D5FF: main (apk-static.c:516)
==31584== 
==31584== Use of uninitialised value of size 8
==31584==    at 0x431118: hlist_add_head (apk_defines.h:234)
==31584==    by 0x4314F4: apk_hash_insert_hashed (hash.c:78)
==31584==    by 0x434FF1: resolve_cache_item (io.c:1074)
==31584==    by 0x435107: apk_resolve_uid (io.c:1112)
==31584==    by 0x43696C: apk_tar_parse (io_archive.c:152)
==31584==    by 0x4271BC: apk_pkg_read (package.c:929)
==31584==    by 0x402D75: add_main (app_add.c:163)
==31584==    by 0x40D5FF: main (apk-static.c:516)
==31584== 
==31584== Warning: invalid file descriptor -1 in syscall read()
==31584== Warning: invalid file descriptor -1 in syscall close()
==31584== Conditional jump or move depends on uninitialised value(s)
==31584==    at 0x5C8CA5: strlen (strlen.c:17)
==31584==    by 0x432575: APK_BLOB_STR (apk_blob.h:79)
==31584==    by 0x435207: apk_resolve_gid (io.c:1154)
==31584==    by 0x4369A9: apk_tar_parse (io_archive.c:153)
==31584==    by 0x4271BC: apk_pkg_read (package.c:929)
==31584==    by 0x402D75: add_main (app_add.c:163)
==31584==    by 0x40D5FF: main (apk-static.c:516)
==31584== 
==31584== Conditional jump or move depends on uninitialised value(s)
==31584==    at 0x5C8CB0: strlen (strlen.c:20)
==31584==    by 0x432575: APK_BLOB_STR (apk_blob.h:79)
==31584==    by 0x435207: apk_resolve_gid (io.c:1154)
==31584==    by 0x4369A9: apk_tar_parse (io_archive.c:153)
==31584==    by 0x4271BC: apk_pkg_read (package.c:929)
==31584==    by 0x402D75: add_main (app_add.c:163)
==31584==    by 0x40D5FF: main (apk-static.c:516)
==31584== 
==31584== Use of uninitialised value of size 8
==31584==    at 0x4313FB: apk_hash_get_hashed (hash.c:61)
==31584==    by 0x434F73: resolve_cache_item (io.c:1064)
==31584==    by 0x435223: apk_resolve_gid (io.c:1154)
==31584==    by 0x4369A9: apk_tar_parse (io_archive.c:153)
==31584==    by 0x4271BC: apk_pkg_read (package.c:929)
==31584==    by 0x402D75: add_main (app_add.c:163)
==31584==    by 0x40D5FF: main (apk-static.c:516)
==31584== 
==31584== Use of uninitialised value of size 8
==31584==    at 0x4310F0: hlist_add_head (apk_defines.h:231)
==31584==    by 0x4314F4: apk_hash_insert_hashed (hash.c:78)
==31584==    by 0x434FF1: resolve_cache_item (io.c:1074)
==31584==    by 0x435223: apk_resolve_gid (io.c:1154)
==31584==    by 0x4369A9: apk_tar_parse (io_archive.c:153)
==31584==    by 0x4271BC: apk_pkg_read (package.c:929)
==31584==    by 0x402D75: add_main (app_add.c:163)
==31584==    by 0x40D5FF: main (apk-static.c:516)
==31584== 
==31584== Use of uninitialised value of size 8
==31584==    at 0x431118: hlist_add_head (apk_defines.h:234)
==31584==    by 0x4314F4: apk_hash_insert_hashed (hash.c:78)
==31584==    by 0x434FF1: resolve_cache_item (io.c:1074)
==31584==    by 0x435223: apk_resolve_gid (io.c:1154)
==31584==    by 0x4369A9: apk_tar_parse (io_archive.c:153)
==31584==    by 0x4271BC: apk_pkg_read (package.c:929)
==31584==    by 0x402D75: add_main (app_add.c:163)
==31584==    by 0x40D5FF: main (apk-static.c:516)
==31584== 
==31584== Warning: invalid file descriptor -1 in syscall read()
==31584== Warning: invalid file descriptor -1 in syscall close()
ERROR: /tmp/out-of-bounds-read-apk-tar-uname.apk: BAD archive
==31584== 
==31584== HEAP SUMMARY:
==31584==     in use at exit: 0 bytes in 0 blocks
==31584==   total heap usage: 0 allocs, 0 frees, 0 bytes allocated
==31584== 
==31584== All heap blocks were freed -- no leaks are possible
==31584== 
==31584== Use --track-origins=yes to see where uninitialised values come from
==31584== For lists of detected and suppressed errors, rerun with: -s
==31584== ERROR SUMMARY: 16 errors from 10 contexts (suppressed: 0 from 0)

Hotfix

diff --git a/src/io_archive.c b/src/io_archive.c
index de4741e..d68263b 100644
--- a/src/io_archive.c
+++ b/src/io_archive.c
@@ -51,6 +51,7 @@ struct tar_header {
 
 #define GET_OCTAL(s)	get_octal(s, sizeof(s))
 #define PUT_OCTAL(s,v)	put_octal(s, sizeof(s), v)
+#define HAS_NULLTERM(a)	memchr(a, '\0', sizeof(a))
 
 static unsigned int get_octal(char *s, size_t l)
 {
@@ -147,6 +148,14 @@ int apk_tar_parse(struct apk_istream *is, apk_archive_entry_parser parser,
 			continue;
 		}
 
+		/* Ensure that fields which should be null-terminated
+		 * are null-terminated to use string functions on them. */
+		if (!HAS_NULLTERM(buf.uname) || !HAS_NULLTERM(buf.gname) ||
+		    !HAS_NULLTERM(buf.linkname) || !HAS_NULLTERM(buf.magic) ||
+		    !HAS_NULLTERM(buf.name)) {
+			goto err;
+		}
+
 		entry = (struct apk_file_info){
 			.size  = GET_OCTAL(buf.size),
 			.uid   = apk_resolve_uid(idc, buf.uname, GET_OCTAL(buf.uid)),