AndreaBozzo opened a new issue, #945:
URL: https://github.com/apache/arrow-nanoarrow/issues/945

   `ArrowArrayViewValidate()` reads offsets buffers through `data.as_int32[i]` 
/ `data.as_int64[i]`. If the buffer isn't aligned to the offset width, that is 
a misaligned load, which is undefined behaviour in C. The C Data Interface 
allows unaligned buffers:
   
   > It is recommended, but not required, that the memory addresses of the 
buffers be aligned at least according to the type of primitive data that they 
contain. Consumers MAY decide not to support unaligned memory.
   
   nanoarrow accepts such arrays, so valid input goes through UB. x86 hides it; 
UBSan reports it.
   
   Sites on `main` (ec8a58c):
   - `ArrowArrayViewValidateDefault()`, first/last offset: 
[string/binary](https://github.com/apache/arrow-nanoarrow/blob/ec8a58cae18beaa241c7fea7cb26816ac27c280f/src/nanoarrow/common/array.c#L1823-L1830),
 
[large_string/large_binary](https://github.com/apache/arrow-nanoarrow/blob/ec8a58cae18beaa241c7fea7cb26816ac27c280f/src/nanoarrow/common/array.c#L1859-L1866)
   - `ArrowAssertIncreasingInt32/Int64()` (full): 
[L2040](https://github.com/apache/arrow-nanoarrow/blob/ec8a58cae18beaa241c7fea7cb26816ac27c280f/src/nanoarrow/common/array.c#L2040),
 
[L2056](https://github.com/apache/arrow-nanoarrow/blob/ec8a58cae18beaa241c7fea7cb26816ac27c280f/src/nanoarrow/common/array.c#L2056)
   - The list/map and large_list branches 
([L1908](https://github.com/apache/arrow-nanoarrow/blob/ec8a58cae18beaa241c7fea7cb26816ac27c280f/src/nanoarrow/common/array.c#L1908-L1915),
 
[L1936](https://github.com/apache/arrow-nanoarrow/blob/ec8a58cae18beaa241c7fea7cb26816ac27c280f/src/nanoarrow/common/array.c#L1936-L1943))
 read offsets the same way; not exercised by the repro below.
   
   ### Reproducer
   
   `repro.c`, at the repository root:
   
   ```c
   #include <stdio.h>
   #include <stdlib.h>
   #include <string.h>
   
   #include "nanoarrow/nanoarrow.h"
   
   int main(void) {
     struct ArrowArray array;
     struct ArrowArrayView view;
     struct ArrowError error;
     struct ArrowStringView values[] = {{"a", 1}, {"bb", 2}, {"ccc", 3}};
   
     ArrowArrayInitFromType(&array, NANOARROW_TYPE_STRING);
     ArrowArrayStartAppending(&array);
     for (int i = 0; i < 3; i++) ArrowArrayAppendString(&array, values[i]);
     ArrowArrayFinishBuildingDefault(&array, &error);
   
     // Same offsets, moved one byte past an int32 boundary
     const void* aligned = array.buffers[1];
     unsigned char* block = malloc(4 * sizeof(int32_t) + 1);
     memcpy(block + 1, aligned, 4 * sizeof(int32_t));
     array.buffers[1] = block + 1;
   
     ArrowArrayViewInitFromType(&view, NANOARROW_TYPE_STRING);
     ArrowArrayViewSetArrayMinimal(&view, &array, &error);
     int code = ArrowArrayViewValidate(&view, NANOARROW_VALIDATION_LEVEL_FULL, 
&error);
     printf("ArrowArrayViewValidate: %d\n", code);
   
     ArrowArrayViewReset(&view);
     array.buffers[1] = aligned;
     ArrowArrayRelease(&array);
     free(block);
     return 0;
   }
   ```
   
   ```sh
   python ci/scripts/bundle.py --output-dir dist
   cc -std=c99 -fsanitize=undefined -Idist/include repro.c dist/src/nanoarrow.c 
-o repro
   ./repro
   ```
   
   ```
   dist/src/nanoarrow.c:4384:65: runtime error: load of misaligned address 
0x... for type 'const int32_t', which requires 4 byte alignment
   dist/src/nanoarrow.c:4391:64: runtime error: load of misaligned address 
0x... for type 'const int32_t', which requires 4 byte alignment
   dist/src/nanoarrow.c:4601:27: runtime error: load of misaligned address 
0x... for type 'const int32_t', which requires 4 byte alignment
   dist/src/nanoarrow.c:4601:51: runtime error: load of misaligned address 
0x... for type 'const int32_t', which requires 4 byte alignment
   ArrowArrayViewValidate: 0
   ```
   
   Expected: no UBSan report. The same happens for `binary`, and for 
`large_string` with the offsets moved by 1 or 4 bytes. It also reproduces on 
0.9.0.
   
   Found by running unaligned C Data Interface buffers through 
`ArrowArrayViewValidate()` under UBSan.
   
   I'm happy to submit a PR. Reading the offsets with `memcpy`, as the metadata 
reader already does, removes all of these reports locally and leaves the 
validation results unchanged.
   


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