diff --git a/tools/aapt2/ResourceValues.cpp b/tools/aapt2/ResourceValues.cpp index e808984c75f53..947e091e2d483 100644 --- a/tools/aapt2/ResourceValues.cpp +++ b/tools/aapt2/ResourceValues.cpp @@ -533,75 +533,119 @@ void Attribute::Print(std::ostream* out) const { } } -static void BuildAttributeMismatchMessage(DiagMessage* msg, - const Attribute* attr, - const Item* value) { - *msg << "expected"; - if (attr->type_mask & android::ResTable_map::TYPE_BOOLEAN) { - *msg << " boolean"; +static void BuildAttributeMismatchMessage(const Attribute& attr, const Item& value, + DiagMessage* out_msg) { + *out_msg << "expected"; + if (attr.type_mask & android::ResTable_map::TYPE_BOOLEAN) { + *out_msg << " boolean"; } - if (attr->type_mask & android::ResTable_map::TYPE_COLOR) { - *msg << " color"; + if (attr.type_mask & android::ResTable_map::TYPE_COLOR) { + *out_msg << " color"; } - if (attr->type_mask & android::ResTable_map::TYPE_DIMENSION) { - *msg << " dimension"; + if (attr.type_mask & android::ResTable_map::TYPE_DIMENSION) { + *out_msg << " dimension"; } - if (attr->type_mask & android::ResTable_map::TYPE_ENUM) { - *msg << " enum"; + if (attr.type_mask & android::ResTable_map::TYPE_ENUM) { + *out_msg << " enum"; } - if (attr->type_mask & android::ResTable_map::TYPE_FLAGS) { - *msg << " flags"; + if (attr.type_mask & android::ResTable_map::TYPE_FLAGS) { + *out_msg << " flags"; } - if (attr->type_mask & android::ResTable_map::TYPE_FLOAT) { - *msg << " float"; + if (attr.type_mask & android::ResTable_map::TYPE_FLOAT) { + *out_msg << " float"; } - if (attr->type_mask & android::ResTable_map::TYPE_FRACTION) { - *msg << " fraction"; + if (attr.type_mask & android::ResTable_map::TYPE_FRACTION) { + *out_msg << " fraction"; } - if (attr->type_mask & android::ResTable_map::TYPE_INTEGER) { - *msg << " integer"; + if (attr.type_mask & android::ResTable_map::TYPE_INTEGER) { + *out_msg << " integer"; } - if (attr->type_mask & android::ResTable_map::TYPE_REFERENCE) { - *msg << " reference"; + if (attr.type_mask & android::ResTable_map::TYPE_REFERENCE) { + *out_msg << " reference"; } - if (attr->type_mask & android::ResTable_map::TYPE_STRING) { - *msg << " string"; + if (attr.type_mask & android::ResTable_map::TYPE_STRING) { + *out_msg << " string"; } - *msg << " but got " << *value; + *out_msg << " but got " << value; } -bool Attribute::Matches(const Item* item, DiagMessage* out_msg) const { +bool Attribute::Matches(const Item& item, DiagMessage* out_msg) const { + constexpr const uint32_t TYPE_ENUM = android::ResTable_map::TYPE_ENUM; + constexpr const uint32_t TYPE_FLAGS = android::ResTable_map::TYPE_FLAGS; + constexpr const uint32_t TYPE_INTEGER = android::ResTable_map::TYPE_INTEGER; + constexpr const uint32_t TYPE_REFERENCE = android::ResTable_map::TYPE_REFERENCE; + android::Res_value val = {}; - item->Flatten(&val); + item.Flatten(&val); + + const uint32_t flattened_data = util::DeviceToHost32(val.data); // Always allow references. - const uint32_t mask = type_mask | android::ResTable_map::TYPE_REFERENCE; - if (!(mask & ResourceUtils::AndroidTypeToAttributeTypeMask(val.dataType))) { + const uint32_t actual_type = ResourceUtils::AndroidTypeToAttributeTypeMask(val.dataType); + + // Only one type must match between the actual and expected. + if ((actual_type & (type_mask | TYPE_REFERENCE)) == 0) { if (out_msg) { - BuildAttributeMismatchMessage(out_msg, this, item); + BuildAttributeMismatchMessage(*this, item, out_msg); } return false; + } - } else if (ResourceUtils::AndroidTypeToAttributeTypeMask(val.dataType) & - android::ResTable_map::TYPE_INTEGER) { - if (static_cast(util::DeviceToHost32(val.data)) < min_int) { + // Enums and flags are encoded as integers, so check them first before doing any range checks. + if ((type_mask & TYPE_ENUM) != 0 && (actual_type & TYPE_ENUM) != 0) { + for (const Symbol& s : symbols) { + if (flattened_data == s.value) { + return true; + } + } + + // If the attribute accepts integers, we can't fail here. + if ((type_mask & TYPE_INTEGER) == 0) { if (out_msg) { - *out_msg << *item << " is less than minimum integer " << min_int; + *out_msg << item << " is not a valid enum"; } return false; - } else if (static_cast(util::DeviceToHost32(val.data)) > max_int) { + } + } + + if ((type_mask & TYPE_FLAGS) != 0 && (actual_type & TYPE_FLAGS) != 0) { + uint32_t mask = 0u; + for (const Symbol& s : symbols) { + mask |= s.value; + } + + // Check if the flattened data is covered by the flag bit mask. + // If the attribute accepts integers, we can't fail here. + if ((mask & flattened_data) == flattened_data) { + return true; + } else if ((type_mask & TYPE_INTEGER) == 0) { if (out_msg) { - *out_msg << *item << " is greater than maximum integer " << max_int; + *out_msg << item << " is not a valid flag"; + } + return false; + } + } + + // Finally check the integer range of the value. + if ((type_mask & TYPE_INTEGER) != 0 && (actual_type & TYPE_INTEGER) != 0) { + if (static_cast(flattened_data) < min_int) { + if (out_msg) { + *out_msg << item << " is less than minimum integer " << min_int; + } + return false; + } else if (static_cast(flattened_data) > max_int) { + if (out_msg) { + *out_msg << item << " is greater than maximum integer " << max_int; } return false; } diff --git a/tools/aapt2/ResourceValues.h b/tools/aapt2/ResourceValues.h index ac5795fb9774a..7e7547fc1b94b 100644 --- a/tools/aapt2/ResourceValues.h +++ b/tools/aapt2/ResourceValues.h @@ -264,7 +264,7 @@ struct Attribute : public BaseValue { Attribute* Clone(StringPool* new_pool) const override; void PrintMask(std::ostream* out) const; void Print(std::ostream* out) const override; - bool Matches(const Item* item, DiagMessage* out_msg) const; + bool Matches(const Item& item, DiagMessage* out_msg = nullptr) const; }; struct Style : public BaseValue @string/wow diff --git a/tools/aapt2/link/ReferenceLinker.cpp b/tools/aapt2/link/ReferenceLinker.cpp index a8e510cd6140b..414e56eb5dccd 100644 --- a/tools/aapt2/link/ReferenceLinker.cpp +++ b/tools/aapt2/link/ReferenceLinker.cpp @@ -109,18 +109,15 @@ class ReferenceLinkerVisitor : public ValueVisitor { entry.value->Accept(this); // Now verify that the type of this item is compatible with the - // attribute it - // is defined for. We pass `nullptr` as the DiagMessage so that this - // check is - // fast and we avoid creating a DiagMessage when the match is - // successful. - if (!symbol->attribute->Matches(entry.value.get(), nullptr)) { + // attribute it is defined for. We pass `nullptr` as the DiagMessage so that this + // check is fast and we avoid creating a DiagMessage when the match is successful. + if (!symbol->attribute->Matches(*entry.value, nullptr)) { // The actual type of this item is incompatible with the attribute. DiagMessage msg(entry.key.GetSource()); // Call the matches method again, this time with a DiagMessage so we // fill in the actual error message. - symbol->attribute->Matches(entry.value.get(), &msg); + symbol->attribute->Matches(*entry.value, &msg); context_->GetDiagnostics()->Error(msg); error_ = true; } diff --git a/tools/aapt2/readme.md b/tools/aapt2/readme.md index 49cb8d4ca8271..62945b1aea8d5 100644 --- a/tools/aapt2/readme.md +++ b/tools/aapt2/readme.md @@ -1,5 +1,9 @@ # Android Asset Packaging Tool 2.0 (AAPT2) release notes +## Version 2.18 +### `aapt2 ...` +- Fixed issue where enum values were interpreted as integers and range checked. (bug 62358540) + ## Version 2.17 ### `aapt2 ...` - Fixed issue where symlinks would not be followed when compiling PNGs. (bug 62144459)