apache / apache/arrow-swift

ArrowReader reads out of bounds when a validity buffer is absent

Open
#186 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Swift
Stars
32
Forks
18
Avg merge
19h 14m
Merged PRs (30d)
2

Description

### Describe the bug, including details regarding any error messages, version, and platform.

## Description

`ArrowData.isNull` can read past the end of its allocation when reading a field whose validity buffer is omitted from the IPC message, producing non-deterministic values.

The Arrow specification allows the validity buffer to be omitted when a field has no nulls, in which case it is written with length 0. Arrow Swift does not currently handle that case.

## How it happens

`makeBuffer` in `ArrowReaderHelper.swift` builds the buffer from the bytes present in the message, but takes `length` from the caller, which is the element count and is independent of how many bytes the message actually carried:

```swift
let bufferData = [UInt8](fileData[startOffset ..< endOffset])
return ArrowBuffer.createBuffer(bufferData, length: length)
```

When the validity buffer is absent, `bufferData` is empty, so in `ArrowBuffer.createBuffer(_:length:)` the capacity becomes `alignTo64(0)`, which is 0, while `length` stays non-zero. The resulting buffer claims a length it has no memory to back.

`ArrowData.isNull` then gates on `length` and reads through `BitUtility.isSet`, which performs an unchecked `load`:

```swift
// ArrowData.swift
return nullBuffer.length > 0 && !BitUtility.isSet(at, buffer: nullBuffer)

// BitUtility.swift
let theByte = buffer.rawPointer.load(fromByteOffset: Int(byteIndex), as: UInt8.self)
```

Separately, `ArrowBuffer.createBuffer(_:length:)` never initializes the memory it allocates, unlike its sibling overload `createBuffer(_:size:doAlign:)`, which zeroes the whole allocation. Any padding between the copied bytes and the 64-byte aligned capacity is therefore uninitialized.

## Reproduction

Reading the same file twice returns different values. Using `cpp-21.0.0/generated_primitive.arrow_file` from `apache/arrow-testing`, where the odd-numbered columns are the `*_nonnullable` fields:

```
run 1: b0c1r0=nil b0c1r2=nil
run 2: b0c1r0=false b0c1r2=true
```

The even-numbered columns, which are the `*_nullable` fields and do carry a validity buffer, are identical between runs.

The same divergence appears with `generated_duplicate_fieldnames.arrow_file`:

```
run 1: c0 name=ints type=int8 nulls=0 r0=93
run 2: c0 name=ints type=int8 nulls=0 r0=nil
```

## Expected behaviour

Per the specification, a field whose validity buffer is omitted has no null values, so `isNull` should return `false` for every index, and no read should occur outside the allocation.

## Impact

Every field written without a validity buffer by another Arrow implementation is affected, on both the file and the stream reader. Values are silently incorrect rather than failing, and the reads are outside the allocated buffer.

Contributor guide

Open the contributing guide

Research direction

Start with makeBuffer in ArrowReaderHelper.swift, then trace ArrowBuffer.createBuffer(_:length:), ArrowData.isNull, and BitUtility.isSet. Reproduce with cpp-21.0.0/generated_primitive.arrow_file and generated_duplicate_fieldnames.arrow_file, and verify that omitted validity buffers produce false for every index without reading beyond the allocation.

Written by the indexing model from the issue text.

Assessment

Tech stack
swift
Domain
data
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.