Skip to content

Commit fe33f86

Browse files
jduckandi34
authored andcommitted
Prevent integer issues in ID3::Iterator::findFrame
Integer overflows could occur a few places within findFrame. These can lead to out-of-bounds reads and potentially infinite loops. Ensure that arithmetic does not wrap around to prevent these behaviors. Bug: 23285192 Change-Id: I72a61df7d5719d1d3f2bd0b37fba86f0f4bbedee
1 parent 5db1e75 commit fe33f86

File tree

1 file changed

+21
-2
lines changed

1 file changed

+21
-2
lines changed

media/libstagefright/id3/ID3.cpp

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -656,6 +656,11 @@ void ID3::Iterator::findFrame() {
656656

657657
mFrameSize += 6;
658658

659+
// Prevent integer overflow in validation
660+
if (SIZE_MAX - mOffset <= mFrameSize) {
661+
return;
662+
}
663+
659664
if (mOffset + mFrameSize > mParent.mSize) {
660665
ALOGV("partial frame at offset %d (size = %d, bytes-remaining = %d)",
661666
mOffset, mFrameSize, mParent.mSize - mOffset - 6);
@@ -685,7 +690,7 @@ void ID3::Iterator::findFrame() {
685690
return;
686691
}
687692

688-
size_t baseSize;
693+
size_t baseSize = 0;
689694
if (mParent.mVersion == ID3_V2_4) {
690695
if (!ParseSyncsafeInteger(
691696
&mParent.mData[mOffset + 4], &baseSize)) {
@@ -695,7 +700,21 @@ void ID3::Iterator::findFrame() {
695700
baseSize = U32_AT(&mParent.mData[mOffset + 4]);
696701
}
697702

698-
mFrameSize = 10 + baseSize;
703+
if (baseSize == 0) {
704+
return;
705+
}
706+
707+
// Prevent integer overflow when adding
708+
if (SIZE_MAX - 10 <= baseSize) {
709+
return;
710+
}
711+
712+
mFrameSize = 10 + baseSize; // add tag id, size field and flags
713+
714+
// Prevent integer overflow in validation
715+
if (SIZE_MAX - mOffset <= mFrameSize) {
716+
return;
717+
}
699718

700719
if (mOffset + mFrameSize > mParent.mSize) {
701720
ALOGV("partial frame at offset %d (size = %d, bytes-remaining = %d)",

0 commit comments

Comments
 (0)