You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avro.apache.org by "OwenSanzas (via GitHub)" <gi...@apache.org> on 2026/01/13 00:35:33 UTC
[PR] [C] Fix negative block size validation in datafile reader [avro]
OwenSanzas opened a new pull request, #3623:
URL: https://github.com/apache/avro/pull/3623
## Summary
Fix missing validation for negative block size values in `file_read_block_count()` function in `datafile.c`.
## Problem
The block size is read using zigzag encoding which can decode to negative numbers from malicious Avro container files. These negative values were passed directly to `avro_malloc()`, causing:
- **allocation-size-too-big** when negative `int64_t` is cast to `size_t`
- Potential crash or undefined behavior when opening malformed `.avro` files
## Changes
- Add `len < 0` check in `file_read_block_count()` before allocation
- Return `EINVAL` with descriptive error message on invalid input
## Testing
Verified with AddressSanitizer fuzzing - crash no longer reproduces.
```
# Before fix:
==ERROR: AddressSanitizer: requested allocation size 0xffffffffffffffff
#1 in file_read_block_count datafile.c:459
# After fix:
No crash - invalid input rejected with error
```
---
Generated with [Claude Code](https://claude.ai/code)
--
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: dev-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [C] Fix negative block size validation in datafile reader [avro]
Posted by "OwenSanzas (via GitHub)" <gi...@apache.org>.
OwenSanzas commented on PR #3623:
URL: https://github.com/apache/avro/pull/3623#issuecomment-3746385334
Thanks for the reply! Here is the context of the crash we found:
## Description
Negative block size in Avro container file (OCF) causes `allocation-size-too-big` crash in `file_read_block_count()`. The function reads block size using varint/zigzag encoding which can represent negative numbers, but does not validate before passing to `avro_malloc()`.
## Version
- Apache Avro C library
- Tested on: current main branch
- Commit: HEAD
## Steps to Reproduce
### Method 1: Using avrocat (Easiest)
**Step 1**: Build Avro C library with AddressSanitizer:
```bash
git clone https://github.com/apache/avro.git
cd avro/lang/c
mkdir build && cd build
cmake .. \
-DCMAKE_C_COMPILER=clang \
-DCMAKE_C_FLAGS="-fsanitize=address -g -O1" \
-DCMAKE_EXE_LINKER_FLAGS="-fsanitize=address"
make -j$(nproc)
```
**Step 2**: Create the malicious Avro container file (83 bytes):
```bash
# One-liner to create poc.avro
echo '4f626a0104166176726f2e736368656d611e7b2274797065223a226e756c6c227d146176726f2e636f646563086e756c6c000102030405060708090a0b0c0d0e0f10000102030405060708090a0b0c0d0e0f10' | xxd -r -p > poc.avro
```
**Step 3**: Trigger the crash:
```bash
./src/avrocat poc.avro
```
### Method 2: Using the fuzzer
**Step 1**: Build Avro C library with AddressSanitizer (same as above).
**Step 2**: Save the fuzzer code below as `datafile_fuzzer.c`.
**Step 3**: Build the fuzzer:
```bash
clang -g -O1 -fsanitize=address,fuzzer \
-I../src \
datafile_fuzzer.c \
-L./src -lavro \
-Wl,-rpath,./src \
-o datafile_fuzzer
```
**Step 4**: Create PoC and run:
```bash
echo '4f626a0104166176726f2e736368656d611e7b2274797065223a226e756c6c227d146176726f2e636f646563086e756c6c000102030405060708090a0b0c0d0e0f10000102030405060708090a0b0c0d0e0f10' | xxd -r -p > poc.avro
./datafile_fuzzer poc.avro
```
## Expected Behavior
The Avro C library should validate that block size is non-negative and return an error for malformed files.
## Actual Behavior
```
==PID==ERROR: AddressSanitizer: requested allocation size 0xffffffffffffffff (0x800 after adjustments for alignment, red zones etc.) exceeds maximum supported size of 0x10000000000 (thread T0)
#0 0x... in realloc (...)
#1 0x... in file_read_block_count /path/to/avro/lang/c/src/datafile.c:459:35
#2 0x... in avro_file_reader_fp /path/to/avro/lang/c/src/datafile.c:529:9
...
SUMMARY: AddressSanitizer: allocation-size-too-big ... in realloc
```
## Root Cause Analysis
In `lang/c/src/datafile.c:452-459`, the `file_read_block_count()` function reads block size using zigzag-encoded varint:
```c
static int file_read_block_count(avro_file_reader_t r)
{
int64_t len;
...
check_prefix(rval, enc->read_long(r->reader, &len),
"Cannot read file block size: ");
if (!r->current_blockdata) {
r->current_blockdata = (char *) avro_malloc(len); // BUG: len can be negative!
...
}
}
```
**Control flow for crash file:**
1. File header parsed successfully (valid "Obj\x01" magic, schema, codec, sync marker)
2. `file_read_block_count()` called at line 529
3. `read_long()` reads block_count from byte 0x00 at offset 0x42 → decoded = 0
4. `read_long()` reads block_size from byte 0x01 at offset 0x43
5. Zigzag decode: `(1 >> 1) ^ -(1 & 1) = 0 ^ -1 = -1`
6. `avro_malloc(-1)` → `avro_malloc(0xFFFFFFFFFFFFFFFF)` → CRASH
**File structure of PoC:**
```
Offset Bytes Description
------ ----- -----------
00-03: 4f 62 6a 01 Magic "Obj\x01"
04-31: ... Metadata map (schema={"type":"null"}, codec=null)
32-41: ... Sync marker (16 bytes)
42: 00 block_count varint (decoded = 0)
43: 01 block_size varint (zigzag decode = -1) <- TRIGGER
```
## Impact
- **DoS**: Application crash via allocation failure
- **CWE-789**: Memory Allocation with Excessive Size Value
- **Affected**: Any application using Avro C library to read untrusted `.avro` files
Attack vectors:
- Data analytics platforms accepting user uploads
- ETL pipelines processing external data
- Message queue consumers (Kafka with Avro)
- Any service that reads Avro container files
## Suggested Fix
> **Note**: This is a quick fix for this specific vulnerability. A comprehensive audit of all `read_long()` call sites that use decoded values for memory allocation is recommended, as similar issues may exist elsewhere in the codebase.
Add validation for negative block size in `file_read_block_count()`:
```c
static int file_read_block_count(avro_file_reader_t r)
{
int64_t len;
...
check_prefix(rval, enc->read_long(r->reader, &len),
"Cannot read file block size: ");
if (len < 0) {
avro_set_error("Invalid block size: %" PRId64, len);
return EINVAL;
}
if (!r->current_blockdata) {
r->current_blockdata = (char *) avro_malloc(len);
...
}
}
```
## Fuzzer
This issue was discovered using a custom Avro datafile fuzzer:
```c
/*
* Copyright 2026 O2Lab @ Texas A&M University
*
* Fuzzer for Avro C DataFile Reader
* Target: avro_file_reader_fp() and avro_file_reader_read_value()
*/
#include <stdint.h>
#include <stddef.h>
#include <string.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <avro.h>
int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
if (size < 4) {
return 0;
}
/* Write fuzz data to a temporary file */
char template[] = "/tmp/avro_fuzz_XXXXXX";
int fd = mkstemp(template);
if (fd < 0) {
return 0;
}
ssize_t written = write(fd, data, size);
if (written != (ssize_t)size) {
close(fd);
unlink(template);
return 0;
}
lseek(fd, 0, SEEK_SET);
FILE *fp = fdopen(fd, "rb");
if (fp == NULL) {
close(fd);
unlink(template);
return 0;
}
avro_file_reader_t reader = NULL;
avro_value_iface_t *iface = NULL;
avro_value_t value;
int rc;
rc = avro_file_reader_fp(fp, template, 0, &reader);
if (rc != 0 || reader == NULL) {
fclose(fp);
unlink(template);
return 0;
}
avro_schema_t schema = avro_file_reader_get_writer_schema(reader);
if (schema == NULL) {
avro_file_reader_close(reader);
fclose(fp);
unlink(template);
return 0;
}
iface = avro_generic_class_from_schema(schema);
if (iface == NULL) {
avro_schema_decref(schema);
avro_file_reader_close(reader);
fclose(fp);
unlink(template);
return 0;
}
memset(&value, 0, sizeof(value));
rc = avro_generic_value_new(iface, &value);
if (rc != 0) {
avro_value_iface_decref(iface);
avro_schema_decref(schema);
avro_file_reader_close(reader);
fclose(fp);
unlink(template);
return 0;
}
/* Read up to 100 values */
for (int i = 0; i < 100; i++) {
rc = avro_file_reader_read_value(reader, &value);
if (rc != 0) {
break;
}
avro_value_reset(&value);
}
avro_value_decref(&value);
avro_value_iface_decref(iface);
avro_schema_decref(schema);
avro_file_reader_close(reader);
fclose(fp);
unlink(template);
return 0;
}
```
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [C] Fix negative block size validation in datafile reader [avro]
Posted by "OwenSanzas (via GitHub)" <gi...@apache.org>.
OwenSanzas commented on code in PR #3623:
URL: https://github.com/apache/avro/pull/3623#discussion_r2688602000
##########
lang/c/src/datafile.c:
##########
@@ -451,6 +451,10 @@ static int file_read_block_count(avro_file_reader_t r)
"Cannot read file block count: ");
check_prefix(rval, enc->read_long(r->reader, &len),
"Cannot read file block size: ");
+ if (len < 0) {
Review Comment:
1. Well I think len==0 is ok here. It seems that
} else if (!r->current_blockdata) {
r->current_blockdata = (char *) avro_malloc(len); // this will return NULL
r->current_blocklen = len;
}
And len == 0 won't break other logic. Also it wont let the reading/decoding happen.
If previous design considered len == 0, I think len < 0 here is enough
I tried len == 0 and it passes some test cases I generated.
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [C] Fix negative block size validation in datafile reader [avro]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3623:
URL: https://github.com/apache/avro/pull/3623#discussion_r2687965545
##########
lang/c/src/datafile.c:
##########
@@ -451,6 +451,10 @@ static int file_read_block_count(avro_file_reader_t r)
"Cannot read file block count: ");
Review Comment:
The block count (`blocks_total`) read from the file should also be validated to ensure it's not negative, similar to the block size validation added below. A negative block count could lead to incorrect behavior in the read logic since `blocks_read` (which starts at 0 and only increments) would never equal a negative `blocks_total`, potentially preventing proper block reading.
```suggestion
"Cannot read file block count: ");
if (r->blocks_total < 0) {
avro_set_error("Invalid block count: %" PRId64, r->blocks_total);
return EINVAL;
}
```
##########
lang/c/src/datafile.c:
##########
@@ -451,6 +451,10 @@ static int file_read_block_count(avro_file_reader_t r)
"Cannot read file block count: ");
check_prefix(rval, enc->read_long(r->reader, &len),
"Cannot read file block size: ");
+ if (len < 0) {
+ avro_set_error("Invalid block size: %" PRId64, len);
+ return EINVAL;
+ }
Review Comment:
Consider adding a regression test that verifies the negative block size validation. While the fix was verified with AddressSanitizer fuzzing, a unit test with a malformed Avro file containing a negative block size would help prevent regressions. This could follow the pattern of other test files like test_avro_1237.c which test handling of malformed Avro files.
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [C] Fix negative block size validation in datafile reader [avro]
Posted by "OwenSanzas (via GitHub)" <gi...@apache.org>.
OwenSanzas commented on PR #3623:
URL: https://github.com/apache/avro/pull/3623#issuecomment-3767001194
@martin-g Thanks for the guidance. You're the hero!
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [C] Fix negative block size validation in datafile reader [avro]
Posted by "OwenSanzas (via GitHub)" <gi...@apache.org>.
OwenSanzas commented on PR #3623:
URL: https://github.com/apache/avro/pull/3623#issuecomment-3747320417
@martin-g All changes are done and passed all test cases. Please LMK if you have any concern.
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [C] Fix negative block size validation in datafile reader [avro]
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on PR #3623:
URL: https://github.com/apache/avro/pull/3623#issuecomment-3766599933
Thank you, @OwenSanzas !
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [C] Fix negative block size validation in datafile reader [avro]
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g merged PR #3623:
URL: https://github.com/apache/avro/pull/3623
--
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: dev-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [C] Fix negative block size validation in datafile reader [avro]
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on code in PR #3623:
URL: https://github.com/apache/avro/pull/3623#discussion_r2688024435
##########
lang/c/src/datafile.c:
##########
@@ -451,6 +451,10 @@ static int file_read_block_count(avro_file_reader_t r)
"Cannot read file block count: ");
check_prefix(rval, enc->read_long(r->reader, &len),
"Cannot read file block size: ");
+ if (len < 0) {
Review Comment:
Is `len == 0` OK here ?
##########
lang/c/src/datafile.c:
##########
@@ -451,6 +451,10 @@ static int file_read_block_count(avro_file_reader_t r)
"Cannot read file block count: ");
check_prefix(rval, enc->read_long(r->reader, &len),
"Cannot read file block size: ");
+ if (len < 0) {
Review Comment:
What if the read value is positive but again a really big one, like INT64_MAX ?
##########
lang/c/src/datafile.c:
##########
@@ -451,6 +451,10 @@ static int file_read_block_count(avro_file_reader_t r)
"Cannot read file block count: ");
check_prefix(rval, enc->read_long(r->reader, &len),
"Cannot read file block size: ");
+ if (len < 0) {
+ avro_set_error("Invalid block size: %" PRId64, len);
+ return EINVAL;
+ }
if (r->current_blockdata && len > r->current_blocklen) {
r->current_blockdata = (char *) avro_realloc(r->current_blockdata, r->current_blocklen, len);
Review Comment:
This is an old code but shouldn't it check that the allocated char* is non-NULL before assigning it to r->current_blockdata ?
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org