Check that the header's section's size are consistent
Check that the whole section (i.e. offset + size) lies
within the file. Otherwise, we risk reading past the
end of the file e.g. in
https://cs.android.com/android/platform/superproject/+/master:art/libdexfile/dex/dex_file_verifier.cc;l=1427;drc=9f5fd34fe52e7bdc7fe2d919c34d366bf5393654.
Bug: 286840166
Fixes: 286840166
Test: SANITIZE_HOST='address' m \
test-art-host-gtest-art_libdexfile_tests64
Change-Id: I84351f97e76ffee4d764fce47c1966e6e1f73897
diff --git a/libdexfile/dex/dex_file_loader_test.cc b/libdexfile/dex/dex_file_loader_test.cc
index 7275032..8abe9a4 100644
--- a/libdexfile/dex/dex_file_loader_test.cc
+++ b/libdexfile/dex/dex_file_loader_test.cc
@@ -275,6 +275,24 @@
"BgAAAFv///8bAQAAAQAAAAAAAG81AQAAARAAAAIAAABwAQAAAiAAABAAAAB+AQAA"
"AwAAKQIAAAA3AgAAACAAAAEAAABDAgAAABAAAAEAAABUAgAA";
+static const char kIncorrectSectionSizeInHeader[] =
+ "ZGV4CjAzOACmfCim6q0xSzKzJXwUA/IZmxR8x10yt8X0AgAAcAAAAHhWNBIwQB8z"
+ "AAAAAFQCAAABAT//cAAAAAcAAACwAAAAAwAAAMwAAAABAACA8AAAAPz/9wD4AAAA"
+ "AQAAOhgBAAC8AzI0AQAAAH4BgAAAAQAAANgAAAApAQAAYQAAACoJWwAxAQAAOgAA"
+ "zAAAAP////8QCQAABAAAADEBAAA6AADMAAAA/////xAJAAAEAAAAAAAAAIAAAAAA"
+ "gAAAAAAAAAEAAADmAAAABAAAAAoAAACAAAAAAIAAAAAAgAAAAICAgAAAAAAAAAAA"
+ "gIAAAAAAgAAAAICAgAAAAAAAAAAAgIAAAAAAAAAAAACAAAAAAHoAAACGgu//Nzk3"
+ "QPUhAEEAAP//dgACAAAADQAAAOYAAAAAAAAAAAAATWFpbm7qYXZhEAD8//cA+AAA"
+ "AAEAADoYAQAAvAMyNAEAAAB+AQAAgIAAAAAAAAAAAICAAAAAAIAAAACAgIAAAAAA"
+ "AAAAAICAAAAAAAAAAAAAgAAAAAB6AAAAhoLv/zc5N0D1IQBBAAD//3YAAgAAAA0A"
+ "AADmAAAAAAAAAAAAAE1haW5u6mF2YRAA/P/3APgAAADYAAAAKQEAAGEAAAAqCVsA"
+ "MQEAADoAAMwAAAD/////EAkAAAQAAAAxAQAAOgAAzAAAAP////8QCQAATWFpbm7q"
+ "YQB2YQFWAAJWGAATizUBAQmQBgwAAAAAAAAAAACHAQAAACAAAAEA+P9t+gAAABAA"
+ "AAEABAAAAICAgACAAAAAAIAABQANAAAAAAAjAAEAAAAAAAAABwAABxAAAABwAAAA"
+ "AgAAAAcAAACwAAAAACAABEEAAADMAAAABAAAAAEnAADwAAAABQAAAwAAAPj4AAAA"
+ "BgAwqDYA//8YAQAAAQAAAAAAAG8zAQAAARAAAAIAAABwAQAAAiAAABAAAAB+AQAA"
+ "AwAAKQIAAAAzAgAABiAAAAEAAABDAgAAABAAAAEAAABUAgAA";
+
static void DecodeDexFile(const char* base64, std::vector<uint8_t>* dex_bytes) {
// decode base64
CHECK(base64 != nullptr);
@@ -575,4 +593,8 @@
OpenAndVerify(kRawBadDebugInfoItem, /*expected_success=*/false);
}
+TEST_F(DexFileLoaderTest, IncorrectSectionSizeInHeader) {
+ OpenAndVerify(kIncorrectSectionSizeInHeader, /*expected_success=*/false);
+}
+
} // namespace art
diff --git a/libdexfile/dex/dex_file_verifier.cc b/libdexfile/dex/dex_file_verifier.cc
index cc1f88c..04cc399 100644
--- a/libdexfile/dex/dex_file_verifier.cc
+++ b/libdexfile/dex/dex_file_verifier.cc
@@ -215,8 +215,11 @@
// Check a list. The head is assumed to be at *ptr, and elements to be of size element_size. If
// successful, the ptr will be moved forward the amount covered by the list.
bool CheckList(size_t element_size, const char* label, const uint8_t* *ptr);
- // Checks whether the offset is zero (when size is zero) or that the offset falls within the area
- // claimed by the file.
+ // Checks:
+ // * the offset is zero (when size is zero),
+ // * the offset falls within the area claimed by the file,
+ // * the offset + size also falls within the area claimed by the file, and
+ // * the alignment of the section
bool CheckValidOffsetAndSize(uint32_t offset, uint32_t size, size_t alignment, const char* label);
// Checks whether the size is less than the limit.
ALWAYS_INLINE bool CheckSizeLimit(uint32_t size, uint32_t limit, const char* label) {
@@ -556,6 +559,13 @@
ErrorStringPrintf("Offset(%d) should be within file size(%zu) for %s.", offset, size_, label);
return false;
}
+ // Check that offset + size is within the file size. Note that we use `<` to allow the section to
+ // end at the same point as the file. Check written as a subtraction to be safe from overfow.
+ if (size_ - offset < size) {
+ ErrorStringPrintf(
+ "Section end(%d) should be within file size(%zu) for %s.", offset + size, size_, label);
+ return false;
+ }
if (alignment != 0 && !IsAlignedParam(offset, alignment)) {
ErrorStringPrintf("Offset(%d) should be aligned by %zu for %s.", offset, alignment, label);
return false;
@@ -607,7 +617,7 @@
/* alignment= */ 0,
"link") &&
CheckValidOffsetAndSize(header_->map_off_,
- header_->map_off_,
+ sizeof(dex::MapList),
/* alignment= */ 4,
"map") &&
CheckValidOffsetAndSize(header_->string_ids_off_,