From 9289a043153435fcb54b57f63b3a5a27565f2a9a Mon Sep 17 00:00:00 2001 From: Ryan Mitchell Date: Wed, 13 Jan 2021 17:21:29 -0800 Subject: [PATCH] Fail idmap parsing earlier if version differs Variable length strings are now in the idmap format. Idmap attempts to parse the old idmap files using the new format and can interpret old data as the length of a variable length string. Fail parsing idmap header early if the idmap magic or version differs from what is expected. Bug: 177295273 Test: boot, apply change, reboot Change-Id: I2914f11547424457fdb78de0ad03976e1fc81084 --- cmds/idmap2/libidmap2/Idmap.cpp | 13 +++++++++-- cmds/idmap2/tests/IdmapTests.cpp | 40 +++++++++++++++++++------------- 2 files changed, 35 insertions(+), 18 deletions(-) diff --git a/cmds/idmap2/libidmap2/Idmap.cpp b/cmds/idmap2/libidmap2/Idmap.cpp index 5af84b0c6e4e7..a0cc407fc9484 100644 --- a/cmds/idmap2/libidmap2/Idmap.cpp +++ b/cmds/idmap2/libidmap2/Idmap.cpp @@ -103,9 +103,18 @@ Result GetPackageCrc(const ZipFile& zip) { std::unique_ptr IdmapHeader::FromBinaryStream(std::istream& stream) { std::unique_ptr idmap_header(new IdmapHeader()); + if (!Read32(stream, &idmap_header->magic_) || !Read32(stream, &idmap_header->version_)) { + return nullptr; + } + + if (idmap_header->magic_ != kIdmapMagic || + idmap_header->version_ != kIdmapCurrentVersion) { + // Do not continue parsing if the file is not a current version idmap. + return nullptr; + } + uint32_t enforce_overlayable; - if (!Read32(stream, &idmap_header->magic_) || !Read32(stream, &idmap_header->version_) || - !Read32(stream, &idmap_header->target_crc_) || !Read32(stream, &idmap_header->overlay_crc_) || + if (!Read32(stream, &idmap_header->target_crc_) || !Read32(stream, &idmap_header->overlay_crc_) || !Read32(stream, &idmap_header->fulfilled_policies_) || !Read32(stream, &enforce_overlayable) || !ReadString(stream, &idmap_header->target_path_) || !ReadString(stream, &idmap_header->overlay_path_) || diff --git a/cmds/idmap2/tests/IdmapTests.cpp b/cmds/idmap2/tests/IdmapTests.cpp index c13b049eadfe6..16b68f01e8f5a 100644 --- a/cmds/idmap2/tests/IdmapTests.cpp +++ b/cmds/idmap2/tests/IdmapTests.cpp @@ -76,6 +76,24 @@ TEST(IdmapTests, CreateIdmapHeaderFromBinaryStream) { ASSERT_EQ(header->GetDebugInfo(), "debug"); } +TEST(IdmapTests, IdmapFailParsingDifferentVersion) { + constexpr size_t kJunkSize = 2000; + std::stringstream stream; + stream << android::kIdmapMagic; + stream << 0xffffffffU; + stream << std::string(kJunkSize, (char) 0xffU); + ASSERT_FALSE(Idmap::FromBinaryStream(stream)); +} + +TEST(IdmapTests, IdmapFailParsingDifferentMagic) { + constexpr size_t kJunkSize = 2000; + std::stringstream stream; + stream << 0xffffffffU; + stream << android::kIdmapCurrentVersion; + stream << std::string(kJunkSize, (char) 0xffU); + ASSERT_FALSE(Idmap::FromBinaryStream(stream)); +} + TEST(IdmapTests, CreateIdmapDataHeaderFromBinaryStream) { const size_t offset = kIdmapRawDataOffset; std::string raw(reinterpret_cast(idmap_raw_data + offset), @@ -425,11 +443,7 @@ TEST(IdmapTests, IdmapHeaderIsUpToDate) { std::stringstream bad_magic_stream(bad_magic_string); std::unique_ptr bad_magic_header = IdmapHeader::FromBinaryStream(bad_magic_stream); - ASSERT_THAT(bad_magic_header, NotNull()); - ASSERT_NE(header->GetMagic(), bad_magic_header->GetMagic()); - ASSERT_FALSE(bad_magic_header->IsUpToDate(target_apk_path, overlay_apk_path, overlay_name, - target_crc, overlay_crc, policies, - /* enforce_overlayable */ true)); + ASSERT_EQ(nullptr, bad_magic_header); // version: bytes (0x4, 0x07) std::string bad_version_string(stream.str()); @@ -440,11 +454,7 @@ TEST(IdmapTests, IdmapHeaderIsUpToDate) { std::stringstream bad_version_stream(bad_version_string); std::unique_ptr bad_version_header = IdmapHeader::FromBinaryStream(bad_version_stream); - ASSERT_THAT(bad_version_header, NotNull()); - ASSERT_NE(header->GetVersion(), bad_version_header->GetVersion()); - ASSERT_FALSE(bad_magic_header->IsUpToDate(target_apk_path, overlay_apk_path, overlay_name, - target_crc, overlay_crc, policies, - /* enforce_overlayable */ true)); + ASSERT_EQ(nullptr, bad_version_header); // target crc: bytes (0x8, 0xb) std::string bad_target_crc_string(stream.str()); @@ -457,7 +467,7 @@ TEST(IdmapTests, IdmapHeaderIsUpToDate) { IdmapHeader::FromBinaryStream(bad_target_crc_stream); ASSERT_THAT(bad_target_crc_header, NotNull()); ASSERT_NE(header->GetTargetCrc(), bad_target_crc_header->GetTargetCrc()); - ASSERT_FALSE(bad_magic_header->IsUpToDate(target_apk_path, overlay_apk_path, overlay_name, + ASSERT_FALSE(bad_target_crc_header->IsUpToDate(target_apk_path, overlay_apk_path, overlay_name, target_crc, overlay_crc, policies, /* enforce_overlayable */ true)); @@ -472,7 +482,7 @@ TEST(IdmapTests, IdmapHeaderIsUpToDate) { IdmapHeader::FromBinaryStream(bad_overlay_crc_stream); ASSERT_THAT(bad_overlay_crc_header, NotNull()); ASSERT_NE(header->GetOverlayCrc(), bad_overlay_crc_header->GetOverlayCrc()); - ASSERT_FALSE(bad_magic_header->IsUpToDate(target_apk_path, overlay_apk_path, overlay_name, + ASSERT_FALSE(bad_overlay_crc_header->IsUpToDate(target_apk_path, overlay_apk_path, overlay_name, target_crc, overlay_crc, policies, /* enforce_overlayable */ true)); @@ -511,7 +521,7 @@ TEST(IdmapTests, IdmapHeaderIsUpToDate) { IdmapHeader::FromBinaryStream(bad_target_path_stream); ASSERT_THAT(bad_target_path_header, NotNull()); ASSERT_NE(header->GetTargetPath(), bad_target_path_header->GetTargetPath()); - ASSERT_FALSE(bad_magic_header->IsUpToDate(target_apk_path, overlay_apk_path, overlay_name, + ASSERT_FALSE(bad_target_path_header->IsUpToDate(target_apk_path, overlay_apk_path, overlay_name, target_crc, overlay_crc, policies, /* enforce_overlayable */ true)); @@ -527,7 +537,7 @@ TEST(IdmapTests, IdmapHeaderIsUpToDate) { target_crc, overlay_crc, policies, /* enforce_overlayable */ true)); - // overlay path: bytes (0x3c, 0x47) + // overlay name: bytes (0x3c, 0x47) std::string bad_overlay_name_string(stream.str()); bad_overlay_name_string[0x3c] = '\0'; std::stringstream bad_overlay_name_stream(bad_overlay_name_string); @@ -538,8 +548,6 @@ TEST(IdmapTests, IdmapHeaderIsUpToDate) { ASSERT_FALSE(bad_overlay_name_header->IsUpToDate(target_apk_path, overlay_apk_path, overlay_name, target_crc, overlay_crc, policies, /* enforce_overlayable */ true)); - - // overlay name: bytes (0x2c, 0x37) } class TestVisitor : public Visitor {