maltzsama opened a new issue, #186:
URL: https://github.com/apache/arrow-swift/issues/186

   ### 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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to