Skip to content

Commit

Permalink
Fix endianness mistakes in ResourceTypes serialization
Browse files Browse the repository at this point in the history
Summary:
This is mostly an exercise in pedantry. If the host machine was not little endian, the serialization code here would likely produce garbage that failed to be read on Android devices.

This diff makes the serialization code abide by a few conventions:
1) Reading header data from a raw pointer should use `dtoh` methods.
2) Feeding data directly into a header struct should be in device order.
3) Parameters to `push_short` and `push_long` helper methods should always be host order, because those methods swap the order.

Differential Revision: D6654271

fbshipit-source-id: 8a52c07f25d00a03bdcc36239b8290908e952194
  • Loading branch information
wsanville authored and facebook-github-bot committed Jan 3, 2018
1 parent db21690 commit 8718ebc
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 22 deletions.
30 changes: 14 additions & 16 deletions libresource/ResourceTypes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -508,26 +508,24 @@ void ResStringPool::serializeWithAdditionalStrings(Vector<char>& cVec) {
}
align_vec(serialized_strings, 4);
// ResChunk_header
auto header_size = mHeader->header.headerSize;
push_short(cVec, mHeader->header.type);
auto header_size = dtohs(mHeader->header.headerSize);
push_short(cVec, RES_STRING_POOL_TYPE);
push_short(cVec, header_size);
// Sum of header size, plus the size of all the String/style data.
auto total_size =
header_size +
string_idx.size() * sizeof(uint32_t) +
serialized_strings.size() * sizeof(char);
auto total_size = header_size + string_idx.size() * sizeof(uint32_t) +
serialized_strings.size() * sizeof(char);
push_long(cVec, total_size);
// ResStringPool_header
auto string_count = string_idx.size();
push_long(cVec, string_count);
push_long(cVec, 0); // style count
// May have wrecked the sort order (not supporting resort right now), so clear
// the bit.
auto flags = mHeader->flags & ~ResStringPool_header::SORTED_FLAG;
auto flags = dtohl(mHeader->flags) & ~ResStringPool_header::SORTED_FLAG;
push_long(cVec, flags);
// strings start
push_long(cVec, header_size + sizeof(uint32_t) * string_count);
// styles start
// styles start
push_long(cVec, 0);
for (const uint32_t &i : string_idx) {
push_long(cVec, i);
Expand Down Expand Up @@ -3274,13 +3272,13 @@ struct ResTable::Package

auto header_size = sizeof(ResTable_package); // should be 288
auto total_size = header_size + serialized_strings.size();
push_short(cVec, package->header.type); // 0x0200
push_short(cVec, RES_TABLE_PACKAGE_TYPE);
push_short(cVec, header_size);
push_long(cVec, total_size);
push_long(cVec, package->id);
push_long(cVec, dtohl(package->id));
auto num_elements = sizeof(package->name) / sizeof(package->name[0]);
for (size_t i = 0; i < num_elements; i++) {
push_short(cVec, package->name[i]);
push_short(cVec, dtohs(package->name[i]));
}
push_long(cVec, header_size); // type strings start (skip over header)
auto last_public_type = dtohl(package->lastPublicType);
Expand All @@ -3291,8 +3289,8 @@ struct ResTable::Package
}
push_long(cVec, last_public_type);
push_long(cVec, header_size + typestr_size); // key strings start
push_long(cVec, package->lastPublicKey);
push_long(cVec, package->typeIdOffset);
push_long(cVec, dtohl(package->lastPublicKey));
push_long(cVec, dtohl(package->typeIdOffset));
cVec.appendVector(serialized_strings);
}

Expand Down Expand Up @@ -7256,7 +7254,7 @@ void ResTable::serializeSingleResType(
}
// Write out the header struct, followed by each offset uint32_t, followed by
// all the entry bytes we already serialized.
push_short(output, 0x201);
push_short(output, RES_TABLE_TYPE_TYPE);
auto type_header_size = sizeof(ResTable_type);
push_short(output, type_header_size);
auto entries_start = type_header_size + offsets.size() * sizeof(uint32_t);
Expand Down Expand Up @@ -7319,8 +7317,8 @@ void ResTable::defineNewType(
// This is a ResTable_typeSpec followed by a ResTable_type.
const auto typespec_header_size = sizeof(ResTable_typeSpec);
const auto typespec_total_size =
typespec_header_size + sizeof(uint32_t) * num_ids;
push_short(output, 0x202); // ResTable_typeSpec identifier
typespec_header_size + sizeof(uint32_t) * num_ids;
push_short(output, RES_TABLE_TYPE_SPEC_TYPE);
push_short(output, typespec_header_size); // header size
push_long(output, typespec_total_size);
output.push_back(type_id);
Expand Down
4 changes: 2 additions & 2 deletions libresource/Serialize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@
}

void push_short(android::Vector<char>& cVec, uint16_t data) {
auto swapped = dtohs(data);
auto swapped = htods(data);
cVec.push_back(swapped);
cVec.push_back(swapped >> 8);
}

void push_long(android::Vector<char>& cVec, uint32_t data) {
auto swapped = dtohl(data);
auto swapped = htodl(data);
cVec.push_back(swapped);
cVec.push_back(swapped >> 8);
cVec.push_back(swapped >> 16);
Expand Down
10 changes: 6 additions & 4 deletions test/unit/ResourceSerializationTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,13 @@ void assert_u16_string(std::u16string actual_str, std::string expected) {
TEST(ResStringPool, AppendToEmptyTable) {
const size_t header_size = sizeof(android::ResStringPool_header);
android::ResStringPool_header header = {
{0x0001, header_size, header_size},
{htods(android::RES_STRING_POOL_TYPE),
htods(header_size),
htodl(header_size)},
0,
0,
android::ResStringPool_header::UTF8_FLAG |
android::ResStringPool_header::SORTED_FLAG,
htodl(android::ResStringPool_header::UTF8_FLAG |
android::ResStringPool_header::SORTED_FLAG),
0,
0};
android::ResStringPool pool((void*)&header, header_size, false);
Expand All @@ -50,7 +52,7 @@ TEST(ResStringPool, AppendToEmptyTable) {
android::ResStringPool after(data, v.size(), false);

// Ensure sort bit was cleared.
auto flags = ((android::ResStringPool_header*)data)->flags;
auto flags = dtohl(((android::ResStringPool_header*)data)->flags);
ASSERT_FALSE(flags & android::ResStringPool_header::SORTED_FLAG);

size_t out_len;
Expand Down

0 comments on commit 8718ebc

Please sign in to comment.