[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, andgnameare null-terminated character strings. All other fields are zero-filled octal numbers in ASCII.
The code for parsing tar entries in apk is here:
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)),