Skip to content

Avro-4193: [c] segfault cannot handle empty byte array #3527

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
steven-aerts wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Avro-4193: [c] segfault cannot handle empty byte array #3527

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lang/c/src/datum.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ static avro_datum_t avro_bytes_private(char *bytes, int64_t size,
avro_datum_t avro_bytes(const char *bytes, int64_t size)
{
char *bytes_copy = (char *) avro_malloc(size);
if (!bytes_copy) {
if (!bytes_copy && size) {
Copy link
Member

@martin-g martin-g Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would happen now if size is 0?
It looks to me that no matter what is the value of bytes_copy it will never enter the if and it will call memcpy(bytes_copy, bytes, 0)

Copy link
Member

@martin-g martin-g Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO the there must be a check for size == 0 as a first step in this function that returns early and avoids any mallocs and memcpys.

avro_set_error("Cannot copy bytes content");
return NULL;
}
Expand Down Expand Up @@ -197,7 +197,7 @@ int avro_bytes_set(avro_datum_t datum, const char *bytes, const int64_t size)
{
int rval;
char *bytes_copy = (char *) avro_malloc(size);
if (!bytes_copy) {
if (!bytes_copy && size) {
Copy link
Member

@martin-g martin-g Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

avro_set_error("Cannot copy bytes content");
return ENOMEM;
}
Expand Down
4 changes: 2 additions & 2 deletions lang/c/src/value-json.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ encode_utf8_bytes(const void *src, size_t src_len,

// Allocate a new buffer for the UTF-8 string and fill it in.
uint8_t *dest8 = (uint8_t *) avro_malloc(utf8_len);
if (dest8 == NULL) {
if (dest8 == NULL && utf8_len) {
avro_set_error("Cannot allocate JSON bytes buffer");
return ENOMEM;
}
Expand Down Expand Up @@ -126,7 +126,7 @@ avro_value_to_json_t(const avro_value_t *value)
return NULL;
}

json_t *result = json_stringn_nocheck((const char *) encoded, encoded_size);
json_t *result = json_stringn_nocheck((const char *) encoded ? encoded : "", encoded_size);
Copy link
Member

@martin-g martin-g Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
json_t *result = json_stringn_nocheck((const char *) encoded ? encoded : "", encoded_size);
json_t *result = json_stringn_nocheck((const char *) encoded ? encoded : "", encoded ? encoded_size : 0);

?

Copy link
Member

@martin-g martin-g Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a similar code for FIXED at https://github.com/steven-aerts/avro/blob/89fb9db03478b793be7188c5683c6560c2eff115/lang/c/src/value-json.c#L244
Does it need the same improvements ?

avro_free(encoded, encoded_size);
if (result == NULL) {
avro_set_error("Cannot allocate JSON bytes");
Expand Down
53 changes: 42 additions & 11 deletions lang/c/tests/test_avro_data.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,18 +40,20 @@ test_allocator(void *ud, void *ptr, size_t osize, size_t nsize)
AVRO_UNUSED(osize);

if (nsize == 0) {
size_t *size = ((size_t *) ptr) - 1;
if (osize != *size) {
fprintf(stderr,
"Error freeing %p:\n"
"Size passed to avro_free (%" PRIsz ") "
"doesn't match size passed to "
"avro_malloc (%" PRIsz ")\n",
ptr, osize, *size);
abort();
//exit(EXIT_FAILURE);
if (ptr) {
size_t *size = ((size_t *) ptr) - 1;
if (osize != *size) {
fprintf(stderr,
"Error freeing %p:\n"
"Size passed to avro_free (%" PRIsz ") "
"doesn't match size passed to "
"avro_malloc (%" PRIsz ")\n",
ptr, osize, *size);
abort();
//exit(EXIT_FAILURE);
}
free(size);
}
free(size);
return NULL;
} else {
size_t real_size = nsize + sizeof(size_t);
Expand Down Expand Up @@ -214,6 +216,34 @@ static int test_bytes(void)
return 0;
}

static int test_empty_bytes(void)
{
char bytes[] = { };
Copy link

Copilot AI Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty array initializer { } is a GNU extension and not standard C. Consider using char *bytes = NULL; or char bytes[1]; for better portability across compilers.

Suggested change
char bytes[] = { };
char *bytes = NULL;

Copilot uses AI. Check for mistakes.
avro_schema_t writer_schema = avro_schema_bytes();
avro_datum_t datum;
avro_datum_t expected_datum;

datum = avro_givebytes(bytes, sizeof(bytes), NULL);
write_read_check(writer_schema, datum, NULL, NULL, "bytes");
test_json(datum, "\"\"");
avro_datum_decref(datum);
avro_schema_decref(writer_schema);

datum = avro_givebytes(NULL, 0, NULL);
avro_givebytes_set(datum, bytes, sizeof(bytes), NULL);
expected_datum = avro_givebytes(bytes, sizeof(bytes), NULL);
if (!avro_datum_equal(datum, expected_datum)) {
fprintf(stderr,
"Expected equal bytes instances.\n");
exit(EXIT_FAILURE);
}
avro_datum_decref(datum);
avro_datum_decref(expected_datum);

avro_schema_decref(writer_schema);
Copy link

Copilot AI Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double free: writer_schema is already decremented at line 230, so this second decrement at line 243 will cause a use-after-free error.

Suggested change
avro_schema_decref(writer_schema);

Copilot uses AI. Check for mistakes.
return 0;
}

static int test_int32(void)
{
int i;
Expand Down Expand Up @@ -657,6 +687,7 @@ int main(void)
{
"string", test_string}, {
"bytes", test_bytes}, {
"empty_bytes", test_empty_bytes}, {
"int", test_int32}, {
"long", test_int64}, {
"float", test_float}, {
Expand Down