Fixed CursorWindow signed math for x86 builds.

All tests for our recent CursorWindow changes have been passing for
ARM 64-bit builds, but they weren't executed against 32-bit x86
builds until after merged.

It's not actually safe to use the "off_t" type, so we need to cast
to "int32_t" when doing checks against possible-negative values,
such as in allocRow().

We also add tests that verify negative rows/columns are identified
as invalid positions, which requires that we check the resulting
pointer against both mSlotsEnd and mSlotsStart.

Bug: 169251528, 171276404, 171275409
Test: atest libandroidfw_tests:CursorWindowTest
Test: atest CtsDatabaseTestCases
Change-Id: Iea5f7546850f691e183fbb6e6d0952cd02b00d0f

2 files changed, 14 insertions, 6 deletions

diff --git a/libs/androidfw/CursorWindow.cpp b/libs/androidfw/CursorWindow.cpp
index 915c0d75a280..1b8db46c54b6 100644
--- a/libs/androidfw/CursorWindow.cpp
+++ b/libs/androidfw/CursorWindow.cpp
@@ -291,11 +291,11 @@ status_t CursorWindow::allocRow() {
         return INVALID_OPERATION;
     }
     size_t size = mNumColumns * kSlotSizeBytes;
-    off_t newOffset = mSlotsOffset - size;
-    if (newOffset < mAllocOffset) {
+    int32_t newOffset = mSlotsOffset - size;
+    if (newOffset < (int32_t) mAllocOffset) {
         maybeInflate();
         newOffset = mSlotsOffset - size;
-        if (newOffset < mAllocOffset) {
+        if (newOffset < (int32_t) mAllocOffset) {
             return NO_MEMORY;
         }
     }
@@ -311,7 +311,7 @@ status_t CursorWindow::freeLastRow() {
         return INVALID_OPERATION;
     }
     size_t size = mNumColumns * kSlotSizeBytes;
-    off_t newOffset = mSlotsOffset + size;
+    size_t newOffset = mSlotsOffset + size;
     if (newOffset > mSize) {
         return NO_MEMORY;
     }
@@ -326,7 +326,7 @@ status_t CursorWindow::alloc(size_t size, uint32_t* outOffset) {
         return INVALID_OPERATION;
     }
     size_t alignedSize = (size + 3) & ~3;
-    off_t newOffset = mAllocOffset + alignedSize;
+    size_t newOffset = mAllocOffset + alignedSize;
     if (newOffset > mSlotsOffset) {
         maybeInflate();
         newOffset = mAllocOffset + alignedSize;
@@ -345,7 +345,7 @@ CursorWindow::FieldSlot* CursorWindow::getFieldSlot(uint32_t row, uint32_t colum
     // see CursorWindow_bench.cpp for more details
     void *result = static_cast<uint8_t*>(mSlotsStart)
             - (((row * mNumColumns) + column) << kSlotShift);
-    if (result < mSlotsEnd || column >= mNumColumns) {
+    if (result < mSlotsEnd || result > mSlotsStart || column >= mNumColumns) {
         LOG(ERROR) << "Failed to read row " << row << ", column " << column
                 << " from a window with " << mNumRows << " rows, " << mNumColumns << " columns";
         return nullptr;
diff --git a/libs/androidfw/tests/CursorWindow_test.cpp b/libs/androidfw/tests/CursorWindow_test.cpp
index dfcf76e6edf6..15be80c48192 100644
--- a/libs/androidfw/tests/CursorWindow_test.cpp
+++ b/libs/androidfw/tests/CursorWindow_test.cpp
@@ -166,6 +166,14 @@ TEST(CursorWindowTest, StoreBounds) {
     ASSERT_EQ(w->getFieldSlot(0, 3), nullptr);
     ASSERT_EQ(w->getFieldSlot(3, 0), nullptr);
     ASSERT_EQ(w->getFieldSlot(3, 3), nullptr);
+
+    // Can't work with invalid indexes
+    ASSERT_NE(w->putLong(-1, 0, 0xcafe), OK);
+    ASSERT_NE(w->putLong(0, -1, 0xcafe), OK);
+    ASSERT_NE(w->putLong(-1, -1, 0xcafe), OK);
+    ASSERT_EQ(w->getFieldSlot(-1, 0), nullptr);
+    ASSERT_EQ(w->getFieldSlot(0, -1), nullptr);
+    ASSERT_EQ(w->getFieldSlot(-1, -1), nullptr);
 }
 
 TEST(CursorWindowTest, Inflate) {