Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
5 changes: 5 additions & 0 deletions lang/c/src/codec.c
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,11 @@ static int decode_snappy(avro_codec_t c, void * data, int64_t len)
uint32_t crc;
size_t outlen;

if (len < 4) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
if (len < 4) {
if (len < 4 || (uint64_t)len > SIZE_MAX) {

len should also be smaller than SIZE_MAX otherwise on 32-bit systems it may overflow when passed to snappy_uncompressed_length() that accepts size_t.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

good point, that 32-bit overflow would slip straight through. added the SIZE_MAX bound to the same guard and pushed. builds fine here with the snappy codec on and the test still passes.

avro_set_error("Snappy block is too small to contain a CRC32 checksum");
return 1;
}

if (snappy_uncompressed_length((const char*)data, len-4, &outlen) != SNAPPY_OK) {
avro_set_error("Uncompressed length error in snappy");
return 1;
Expand Down
1 change: 1 addition & 0 deletions lang/c/tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,4 @@ add_avro_test_checkmem(test_avro_1691)
add_avro_test_checkmem(test_avro_1906)
add_avro_test_checkmem(test_avro_1904)
add_avro_test_checkmem(test_avro_4246)
add_avro_test_checkmem(test_avro_3807)
87 changes: 87 additions & 0 deletions lang/c/tests/test_avro_3807.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to you under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
* implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

#include "avro.h"
#include "avro_private.h"
#include "codec.h"
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

/*
* A snappy block carries a four byte CRC32 trailer, so anything shorter
* than that cannot be a valid block. The length comes straight from the
* container file and decode_snappy used to compute len - 4 before checking
* it, which underflows for blocks of 0 to 3 bytes and reads out of bounds.
* Make sure those undersized blocks are rejected rather than decoded.
*/

int main(void)
{
#ifdef SNAPPY_CODEC
int i;

avro_codec_t codec = (avro_codec_t) avro_new(struct avro_codec_t_);
if (codec == NULL) {
fprintf(stderr, "Cannot allocate codec\n");
return EXIT_FAILURE;
}

if (avro_codec(codec, "snappy") != 0) {
fprintf(stderr, "Cannot create snappy codec: %s\n",
avro_strerror());
avro_freet(struct avro_codec_t_, codec);
return EXIT_FAILURE;
}

/*
* Use an exact sized heap buffer for every length so a checker such as
* valgrind catches any read past it. The bytes have the varint
* continuation bit set so that, without the guard, the underflowed
* len-4 lets snappy keep reading the length prefix off the end of the
* block.
*/
for (i = 0; i < 4; i++) {
size_t size = (i == 0) ? 1 : (size_t) i;
char *buf = (char *) avro_malloc(size);
if (buf == NULL) {
avro_codec_reset(codec);
avro_freet(struct avro_codec_t_, codec);
return EXIT_FAILURE;
}
memset(buf, 0xff, size);

if (avro_codec_decode(codec, buf, i) == 0) {
fprintf(stderr,
"snappy block of %d bytes should be rejected\n",
i);
avro_free(buf, size);
avro_codec_reset(codec);
avro_freet(struct avro_codec_t_, codec);
return EXIT_FAILURE;
}

avro_free(buf, size);
}

avro_codec_reset(codec);
avro_freet(struct avro_codec_t_, codec);
#else
fprintf(stderr, "Snappy codec not available, skipping\n");
#endif
return EXIT_SUCCESS;
}
Loading