From 85b3e879f856a99e383bf81ac7ce649a86ada772 Mon Sep 17 00:00:00 2001 From: Yurii Zubrytskyi Date: Mon, 29 Nov 2021 23:55:24 -0800 Subject: Add missing size check when parsing staged aliases Need to have the same kind of data size check as in other types parsing Bug: 203938029 Test: manual Change-Id: I9f5d2851ff59da90163ead6c0416f0bba3868cc4 Merged-In: I9f5d2851ff59da90163ead6c0416f0bba3868cc4 (cherry picked from commit 8002034e6b11e9be85671505475936b1ec3705b3) --- libs/androidfw/LoadedArsc.cpp | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'libs/androidfw/LoadedArsc.cpp') diff --git a/libs/androidfw/LoadedArsc.cpp b/libs/androidfw/LoadedArsc.cpp index d17c32817994..8150e78fdddc 100644 --- a/libs/androidfw/LoadedArsc.cpp +++ b/libs/androidfw/LoadedArsc.cpp @@ -686,6 +686,12 @@ std::unique_ptr LoadedPackage::Load(const Chunk& chunk, std::unordered_set finalized_ids; const auto lib_alias = child_chunk.header(); if (!lib_alias) { + LOG(ERROR) << "RES_TABLE_STAGED_ALIAS_TYPE is too small."; + return {}; + } + if ((child_chunk.data_size() / sizeof(ResTable_staged_alias_entry)) + < dtohl(lib_alias->count)) { + LOG(ERROR) << "RES_TABLE_STAGED_ALIAS_TYPE is too small to hold entries."; return {}; } const auto entry_begin = child_chunk.data_ptr().convert(); -- cgit v1.2.3-59-g8ed1b From 74f78adb63ece5ee4827528cb5c1a0302e9ac9ce Mon Sep 17 00:00:00 2001 From: Jason O'Brien Date: Thu, 26 Aug 2021 19:07:47 -0500 Subject: Fix segfault with sparse encoding The native code that underlies `Resources#getIdentifier()` did not take sparse encoding into account when calculating offsets, resulting in either garbage or SEGV_MAPERR whenever a resource is first encountered in a sparsely encoded configuration. Bug: 197976367 Test: atest libandroidfw_tests --host Change-Id: Ib7550fe2e05005550f59129a06be5712b74bc9c8 (cherry picked from commit 984e897303519e02192a77aef372c2a7f53aca6b) --- libs/androidfw/LoadedArsc.cpp | 13 +++++++-- libs/androidfw/tests/LoadedArsc_test.cpp | 32 +++++++++++++++++++++ libs/androidfw/tests/data/sparse/R.h | 19 ++++++------ libs/androidfw/tests/data/sparse/gen_strings.sh | 2 ++ libs/androidfw/tests/data/sparse/not_sparse.apk | Bin 61971 -> 62155 bytes .../tests/data/sparse/res/values-v26/strings.xml | 1 + libs/androidfw/tests/data/sparse/sparse.apk | Bin 59275 -> 59459 bytes 7 files changed, 56 insertions(+), 11 deletions(-) (limited to 'libs/androidfw/LoadedArsc.cpp') diff --git a/libs/androidfw/LoadedArsc.cpp b/libs/androidfw/LoadedArsc.cpp index d17c32817994..446e580d7e9d 100644 --- a/libs/androidfw/LoadedArsc.cpp +++ b/libs/androidfw/LoadedArsc.cpp @@ -384,7 +384,16 @@ base::expected LoadedPackage::FindEntryByName( return base::unexpected(IOError::PAGES_MISSING); } - auto offset = dtohl(entry_offset_ptr.value()); + uint32_t offset; + uint16_t res_idx; + if (type->flags & ResTable_type::FLAG_SPARSE) { + auto sparse_entry = entry_offset_ptr.convert(); + offset = dtohs(sparse_entry->offset) * 4u; + res_idx = dtohs(sparse_entry->idx); + } else { + offset = dtohl(entry_offset_ptr.value()); + res_idx = entry_idx; + } if (offset != ResTable_type::NO_ENTRY) { auto entry = type.offset(dtohl(type->entriesStart) + offset).convert(); if (!entry) { @@ -394,7 +403,7 @@ base::expected LoadedPackage::FindEntryByName( if (dtohl(entry->key.index) == static_cast(*key_idx)) { // The package ID will be overridden by the caller (due to runtime assignment of package // IDs for shared libraries). - return make_resid(0x00, *type_idx + type_id_offset_ + 1, entry_idx); + return make_resid(0x00, *type_idx + type_id_offset_ + 1, res_idx); } } } diff --git a/libs/androidfw/tests/LoadedArsc_test.cpp b/libs/androidfw/tests/LoadedArsc_test.cpp index f356c8130080..d214e2dfef7b 100644 --- a/libs/androidfw/tests/LoadedArsc_test.cpp +++ b/libs/androidfw/tests/LoadedArsc_test.cpp @@ -95,6 +95,38 @@ TEST(LoadedArscTest, LoadSparseEntryApp) { ASSERT_TRUE(LoadedPackage::GetEntry(type.type, entry_index).has_value()); } +TEST(LoadedArscTest, FindSparseEntryApp) { + std::string contents; + ASSERT_TRUE(ReadFileFromZipToString(GetTestDataPath() + "/sparse/sparse.apk", "resources.arsc", + &contents)); + + std::unique_ptr loaded_arsc = LoadedArsc::Load(contents.data(), + contents.length()); + ASSERT_THAT(loaded_arsc, NotNull()); + + const LoadedPackage* package = + loaded_arsc->GetPackageById(get_package_id(sparse::R::string::only_v26)); + ASSERT_THAT(package, NotNull()); + + const uint8_t type_index = get_type_id(sparse::R::string::only_v26) - 1; + const uint16_t entry_index = get_entry_id(sparse::R::string::only_v26); + + const TypeSpec* type_spec = package->GetTypeSpecByTypeIndex(type_index); + ASSERT_THAT(type_spec, NotNull()); + ASSERT_THAT(type_spec->type_entries.size(), Ge(1u)); + + // Ensure that AAPT2 sparsely encoded the v26 config as expected. + auto type_entry = std::find_if( + type_spec->type_entries.begin(), type_spec->type_entries.end(), + [](const TypeSpec::TypeEntry& x) { return x.config.sdkVersion == 26; }); + ASSERT_NE(type_entry, type_spec->type_entries.end()); + ASSERT_NE(type_entry->type->flags & ResTable_type::FLAG_SPARSE, 0); + + // Test fetching a resource with only sparsely encoded configs by name. + auto id = package->FindEntryByName(u"string", u"only_v26"); + ASSERT_EQ(id.value(), fix_package_id(sparse::R::string::only_v26, 0)); +} + TEST(LoadedArscTest, LoadSharedLibrary) { std::string contents; ASSERT_TRUE(ReadFileFromZipToString(GetTestDataPath() + "/lib_one/lib_one.apk", "resources.arsc", diff --git a/libs/androidfw/tests/data/sparse/R.h b/libs/androidfw/tests/data/sparse/R.h index 243e74fac65a..2492dbf33f4a 100644 --- a/libs/androidfw/tests/data/sparse/R.h +++ b/libs/androidfw/tests/data/sparse/R.h @@ -27,21 +27,22 @@ struct R { struct integer { enum : uint32_t { foo_0 = 0x7f010000, - foo_1 = 0x7f010000, - foo_2 = 0x7f010000, - foo_3 = 0x7f010000, - foo_4 = 0x7f010000, - foo_5 = 0x7f010000, - foo_6 = 0x7f010000, - foo_7 = 0x7f010000, - foo_8 = 0x7f010000, - foo_9 = 0x7f010000, + foo_1 = 0x7f010001, + foo_2 = 0x7f010002, + foo_3 = 0x7f010003, + foo_4 = 0x7f010004, + foo_5 = 0x7f010005, + foo_6 = 0x7f010006, + foo_7 = 0x7f010007, + foo_8 = 0x7f010008, + foo_9 = 0x7f010009, }; }; struct string { enum : uint32_t { foo_999 = 0x7f0203e7, + only_v26 = 0x7f0203e8 }; }; }; diff --git a/libs/androidfw/tests/data/sparse/gen_strings.sh b/libs/androidfw/tests/data/sparse/gen_strings.sh index e7e1d603ea4e..4ea5468c7df9 100755 --- a/libs/androidfw/tests/data/sparse/gen_strings.sh +++ b/libs/androidfw/tests/data/sparse/gen_strings.sh @@ -14,5 +14,7 @@ do fi done echo "" >> $OUTPUT_default + +echo " only v26" >> $OUTPUT_v26 echo "" >> $OUTPUT_v26 diff --git a/libs/androidfw/tests/data/sparse/not_sparse.apk b/libs/androidfw/tests/data/sparse/not_sparse.apk index 599a370dbfb1..b08a621195c0 100644 Binary files a/libs/androidfw/tests/data/sparse/not_sparse.apk and b/libs/androidfw/tests/data/sparse/not_sparse.apk differ diff --git a/libs/androidfw/tests/data/sparse/res/values-v26/strings.xml b/libs/androidfw/tests/data/sparse/res/values-v26/strings.xml index b6f82997d18b..d116087ec3c0 100644 --- a/libs/androidfw/tests/data/sparse/res/values-v26/strings.xml +++ b/libs/androidfw/tests/data/sparse/res/values-v26/strings.xml @@ -333,4 +333,5 @@ 9930 9960 9990 + only v26 diff --git a/libs/androidfw/tests/data/sparse/sparse.apk b/libs/androidfw/tests/data/sparse/sparse.apk index 1f9bba31b0a1..9fd01fbf2941 100644 Binary files a/libs/androidfw/tests/data/sparse/sparse.apk and b/libs/androidfw/tests/data/sparse/sparse.apk differ -- cgit v1.2.3-59-g8ed1b