Skip to content

Commit

Permalink
Check bounds in offsetToPtr
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Fyodor Kupolov committed Feb 16, 2017
1 parent 3d52f79 commit 45e2e95
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 3 deletions.
5 changes: 5 additions & 0 deletions libs/androidfw/CursorWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,14 @@ status_t CursorWindow::createFromParcel(Parcel* parcel, CursorWindow** outCursor
if (dupAshmemFd < 0) {
result = -errno;
} else {
// the size of the ashmem descriptor can be modified between ashmem_get_size_region
// call and mmap, so we'll check again immediately after memory is mapped
void* data = ::mmap(NULL, size, PROT_READ, MAP_SHARED, dupAshmemFd, 0);
if (data == MAP_FAILED) {
result = -errno;
} else if (ashmem_get_size_region(dupAshmemFd) != size) {
::munmap(data, size);
result = BAD_VALUE;
} else {
CursorWindow* window = new CursorWindow(name, dupAshmemFd,
data, size, true /*readOnly*/);
Expand Down
17 changes: 14 additions & 3 deletions libs/androidfw/include/androidfw/CursorWindow.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#ifndef _ANDROID__DATABASE_WINDOW_H
#define _ANDROID__DATABASE_WINDOW_H

#include <inttypes.h>
#include <stddef.h>
#include <stdint.h>

Expand Down Expand Up @@ -128,12 +129,13 @@ class CursorWindow {
inline const char* getFieldSlotValueString(FieldSlot* fieldSlot,
size_t* outSizeIncludingNull) {
*outSizeIncludingNull = fieldSlot->data.buffer.size;
return static_cast<char*>(offsetToPtr(fieldSlot->data.buffer.offset));
return static_cast<char*>(offsetToPtr(
fieldSlot->data.buffer.offset, fieldSlot->data.buffer.size));
}

inline const void* getFieldSlotValueBlob(FieldSlot* fieldSlot, size_t* outSize) {
*outSize = fieldSlot->data.buffer.size;
return offsetToPtr(fieldSlot->data.buffer.offset);
return offsetToPtr(fieldSlot->data.buffer.offset, fieldSlot->data.buffer.size);
}

private:
Expand Down Expand Up @@ -166,7 +168,16 @@ class CursorWindow {
bool mReadOnly;
Header* mHeader;

inline void* offsetToPtr(uint32_t offset) {
inline void* offsetToPtr(uint32_t offset, uint32_t bufferSize = 0) {
if (offset >= mSize) {
ALOGE("Offset %" PRIu32 " out of bounds, max value %zu", offset, mSize);
return NULL;
}
if (offset + bufferSize > mSize) {
ALOGE("End offset %" PRIu32 " out of bounds, max value %zu",
offset + bufferSize, mSize);
return NULL;
}
return static_cast<uint8_t*>(mData) + offset;
}

Expand Down

0 comments on commit 45e2e95

Please sign in to comment.