Skip to content

Avoid signed overflow in AndroidZoneInfoSource::Open offset math#360

Closed
ravi0800 wants to merge 1 commit into
google:masterfrom
ravi0800:android-tzdata-offset-overflow
Closed

Avoid signed overflow in AndroidZoneInfoSource::Open offset math#360
ravi0800 wants to merge 1 commit into
google:masterfrom
ravi0800:android-tzdata-offset-overflow

Conversation

@ravi0800

Copy link
Copy Markdown
Contributor

AndroidZoneInfoSource::Open() computes a zone's file position as data_offset + Decode32(ebuf + 40), where both operands are int_fast32_t values pulled straight from the tzdata index. On a build where int_fast32_t is a 32-bit type (32-bit Android/ARM), two near-INT_MAX index values sum past the type's range, so the addition is signed overflow before the start < 0 guard on the next line ever runs. UBSan on a crafted tzdata blob reports "signed integer overflow: 76 + 2147483647 cannot be represented in type int" right at that add.

Widen the left operand to int_fast64_t so the sum is formed in a type that holds it, which leaves the existing start < 0 / length < 0 rejection meaningful and the later static_cast(start) for fseek unchanged. Doing the widening at the add keeps the fix inside the decode where the operand widths are still in view.

Comment thread src/time_zone_info.cc
Comment on lines -536 to +537
const std::int_fast32_t start = data_offset + Decode32(ebuf + 40);
const std::int_fast64_t start =
std::int_fast64_t{data_offset} + Decode32(ebuf + 40);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it is being handled in #356. Please take a look there and see if you have anything to add. Otherwise we can close this PR. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants