cactus-compute / cactus-compute/cactus
RN Android indexAdd skips the input validation iOS performs — ragged embeddings cause an out-of-bounds read
- Dominant language
- C++
- Stars
- 6k
- Forks
- 501
- Avg merge
- 1d 18h
- Merged PRs (30d)
- 4
Description
The React Native bindings validate `indexAdd` inputs on iOS but not on Android, so identical JS produces a clean rejection on one platform and an out-of-bounds read on the other.
`bindings/react-native/apple/Cactus.swift` guards two things:
- L374 — `ids` / `documents` / `embeddings` / `metadatas` must be equal length
- L379 — all embedding rows must be the same length
`bindings/react-native/android/CactusModule.kt` (L338) has neither.
## Repro
```js
await Cactus.indexAdd(handle, [1, 2], ["a", "b"], [[1, 2, 3], [4, 5]], null);
```
- **iOS** — rejects with "Embedding rows must all have the same length"
- **Android** — `embeddingDim` is taken from row 0 (`CactusModule.kt:351`), so it becomes 3. Both rows are non-null, so the JNI loop sets `acquired == count` and calls through. `cactus_index_add` is then told `embedding_dim = 3` and reads 3 floats out of a 2-float row.
## Why the JNI layer doesn't catch it
`android/cactus_jni.cpp:417` derives `count` from `ids` and the loop only breaks on *null* elements. It never compares `GetArrayLength(embeddings[i])` against `embeddingDim`, so a short-but-present row passes every check.
Length *mismatches* are survivable for the same reason — `count` comes from `ids`, the loop breaks early, and `-1` is returned. So this report is specifically about row width. The mismatch case is only a worse error message on Android than on iOS.
## Also
`readableNestedFloatArrays` (`CactusModule.kt:59-61`) throws `IllegalArgumentException` on a null row. That's uncaught inside a `@ReactMethod`, where iOS would reject the promise.
## Note
Found by inspection while comparing the two platform bindings — I don't have an Android device set up to attach a crash log.
Happy to PR this: mirroring the two Swift guards plus a try/catch around the helper, using the existing `fail()` so the error strings match iOS.
Contributor guide
Research direction
Start by comparing the guards at the cited lines in bindings/react-native/apple/Cactus.swift and bindings/react-native/android/CactusModule.kt, then trace android/cactus_jni.cpp:417 and the readableNestedFloatArrays helper. Reproduce the ragged-row and null-row cases through the shown React Native call. Done means Android rejects invalid inputs safely with errors matching iOS instead of reaching the JNI read.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp, kotlin, react-native, swift
- Domain
- mobile
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100