KyleKim107 opened a new issue, #554:
URL: https://github.com/apache/avro-rs/issues/554
# `avro_generic_map_class` is calling the wrong schema accessor (looks like
a copy-paste bug)
Hey — I think I found a small copy-paste bug in the C library.
## What's going on
In `lang/c/src/generic.c`, `avro_generic_map_class()` calls
`avro_schema_array_items()` on a **map** schema. Pretty sure it should be
calling `avro_schema_map_values()` instead.
`avro_generic_array_class()` right above it does the right thing:
```c
avro_schema_t child_schema = avro_schema_array_items(schema); // fine for
arrays
```
But the map version copied the same line:
```c
avro_schema_t child_schema = avro_schema_array_items(schema); // wrong for
maps
```
Other map code in the repo already uses the right helper — e.g.
`datum_value.c` does `avro_schema_map_values(map_schema)`.
## Why you might not have noticed
Right now it kinda works anyway, because `avro_array_schema_t` and
`avro_map_schema_t` have the same layout:
```c
struct avro_array_schema_t { struct avro_obj_t obj; avro_schema_t items; };
struct avro_map_schema_t { struct avro_obj_t obj; avro_schema_t values; };
```
Same offset for `items` / `values`, so you get the right pointer by accident
in a normal build. But it's still UB — you're casting a map schema to an array
schema via `container_of`, and UBSan catches it.
If someone ever adds a field to one struct but not the other, this would
break silently in prod. Not great.
**Where:** `lang/c/src/generic.c`, `avro_generic_map_class()` (~line 2781 on
current `main`)
## How to repro
You don't need malicious binary input or anything fancy. Just build with
UBSan and create a map schema.
**Build avro with UBSan:**
```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,undefined
-fno-sanitize-recover=undefined -g -O1"
make -j$(nproc) avro-static
```
**`repro.c`:**
```c
#include <avro.h>
#include <stdio.h>
#include <string.h>
int main(void)
{
const char *schema_json = "{\"type\": \"map\", \"values\": \"string\"}";
avro_schema_t schema = NULL;
avro_value_iface_t *iface = NULL;
avro_value_t value;
if (avro_schema_from_json_length(schema_json, strlen(schema_json),
&schema) != 0) {
fprintf(stderr, "schema parse failed\n");
return 1;
}
/* hits avro_generic_map_class() → avro_schema_array_items() */
iface = avro_generic_class_from_schema(schema);
if (iface == NULL) {
fprintf(stderr, "iface is NULL\n");
avro_schema_decref(schema);
return 1;
}
memset(&value, 0, sizeof(value));
avro_generic_value_new(iface, &value);
avro_value_decref(&value);
avro_value_iface_decref(iface);
avro_schema_decref(schema);
return 0;
}
```
**Run it:**
```bash
clang -fsanitize=address,undefined -fno-sanitize-recover=undefined -g -O1 \
-I../src repro.c -L./src -lavro $(pkg-config --libs jansson) \
-Wl,-rpath,./src -o repro
ASAN_OPTIONS=detect_leaks=0 UBSAN_OPTIONS=print_stacktrace=1 ./repro
```
**What you'll see (UBSan build):**
```
lang/c/src/schema.c:462:9: runtime error: member access within null pointer
of type 'struct avro_array_schema_t'
#0 avro_schema_array_items
#1 avro_generic_class_from_schema_memoized
#2 avro_generic_class_from_schema
#3 main
```
Exact message might vary a bit depending on compiler/opt level, but it's
always the map class construction path.
**Sanity check:** rebuild without UBSan and run the same program — it
finishes fine. So this isn't blowing up prod today, it's just wrong code that
UBSan happens to catch.
## Impact (being honest about it)
This isn't a security bug as far as I can tell — no memory corruption, no
crash in normal builds. It's more of a "this is technically UB and will bite
you later" kind of thing. UBSan/fuzzing builds will abort on it though.
## Suggested fix
One line:
```diff
static avro_generic_value_iface_t *
avro_generic_map_class(avro_schema_t schema, memoize_state_t *state)
{
- avro_schema_t child_schema = avro_schema_array_items(schema);
+ avro_schema_t child_schema = avro_schema_map_values(schema);
avro_generic_value_iface_t *child_giface =
avro_generic_class_from_schema_memoized(child_schema, state);
```
I tried this locally and the repro passes cleanly under UBSan after the
change.
## How I found it
Fuzzing `avro_generic_class_from_schema()` / `avro_generic_value_new()` with
UBSan on. Crashes as soon as the fuzzer picks a map schema — before any binary
payload gets read.
Happy to open a PR if that's helpful.
--
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]
