Skip to content

Commit 6a54f76

Browse files
Fyodor Kupolovandi34
authored andcommitted
[DO NOT MERGE] Check bounds in offsetToPtr
Check whether specified offset belongs to mData. Also added a default argument bufferSize to check the end offset. Size of the ashmem descriptor can be modified between ashmem_get_size_region call and mmap. createFromParcel method was updated to check ashmem size again immediately after memory is mapped. Test: manual - using the test app from the bug Bug: 34128677 Change-Id: I3ecd1616a870ce20941ce9b20a1843d2b4295750 (cherry picked from commit 45e2e95) (cherry picked from commit acede24)
1 parent a8214b5 commit 6a54f76

File tree

2 files changed

+19
-3
lines changed

2 files changed

+19
-3
lines changed

include/androidfw/CursorWindow.h

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#define _ANDROID__DATABASE_WINDOW_H
1919

2020
#include <cutils/log.h>
21+
#include <inttypes.h>
2122
#include <stddef.h>
2223
#include <stdint.h>
2324

@@ -128,12 +129,13 @@ class CursorWindow {
128129
inline const char* getFieldSlotValueString(FieldSlot* fieldSlot,
129130
size_t* outSizeIncludingNull) {
130131
*outSizeIncludingNull = fieldSlot->data.buffer.size;
131-
return static_cast<char*>(offsetToPtr(fieldSlot->data.buffer.offset));
132+
return static_cast<char*>(offsetToPtr(
133+
fieldSlot->data.buffer.offset, fieldSlot->data.buffer.size));
132134
}
133135

134136
inline const void* getFieldSlotValueBlob(FieldSlot* fieldSlot, size_t* outSize) {
135137
*outSize = fieldSlot->data.buffer.size;
136-
return offsetToPtr(fieldSlot->data.buffer.offset);
138+
return offsetToPtr(fieldSlot->data.buffer.offset, fieldSlot->data.buffer.size);
137139
}
138140

139141
private:
@@ -166,7 +168,16 @@ class CursorWindow {
166168
bool mReadOnly;
167169
Header* mHeader;
168170

169-
inline void* offsetToPtr(uint32_t offset) {
171+
inline void* offsetToPtr(uint32_t offset, uint32_t bufferSize = 0) {
172+
if (offset >= mSize) {
173+
ALOGE("Offset %lu out of bounds, max value %zu", offset, mSize);
174+
return NULL;
175+
}
176+
if (offset + bufferSize > mSize) {
177+
ALOGE("End offset %lu out of bounds, max value %zu",
178+
offset + bufferSize, mSize);
179+
return NULL;
180+
}
170181
return static_cast<uint8_t*>(mData) + offset;
171182
}
172183

libs/androidfw/CursorWindow.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,9 +98,14 @@ status_t CursorWindow::createFromParcel(Parcel* parcel, CursorWindow** outCursor
9898
if (dupAshmemFd < 0) {
9999
result = -errno;
100100
} else {
101+
// the size of the ashmem descriptor can be modified between ashmem_get_size_region
102+
// call and mmap, so we'll check again immediately after memory is mapped
101103
void* data = ::mmap(NULL, size, PROT_READ, MAP_SHARED, dupAshmemFd, 0);
102104
if (data == MAP_FAILED) {
103105
result = -errno;
106+
} else if (ashmem_get_size_region(dupAshmemFd) != size) {
107+
::munmap(data, size);
108+
result = BAD_VALUE;
104109
} else {
105110
CursorWindow* window = new CursorWindow(name, dupAshmemFd,
106111
data, size, true /*readOnly*/);

0 commit comments

Comments
 (0)