reftable: fix resource leak in block.c error path
Add test coverage for corrupt zlib data. Fix memory leaks demonstrated by unittest. This problem was discovered by a Coverity scan. Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Han-Wen Nienhuys committed
Jan 20, 2022 at 15:12 UTC
24d4d38c0b32fd4eb075af89d7b744c5647db5c2
3 files changed
+97
-18
reftable/block.c
+17
-9
@@ -188,13 +188,16 @@ int block_reader_init(struct block_reader *br, struct reftable_block *block,
188
uint32_t full_block_size = table_block_size;
189
uint8_t typ = block->data[header_off];
190
uint32_t sz = get_be24(block->data + header_off + 1);
191
-
191
+ int err = 0;
192
uint16_t restart_count = 0;
193
uint32_t restart_start = 0;
194
uint8_t *restart_bytes = NULL;
195
+ uint8_t *uncompressed = NULL;
196
196
- if (!reftable_is_block_type(typ))
197
- return REFTABLE_FORMAT_ERROR;
197
+ if (!reftable_is_block_type(typ)) {
198
+ err = REFTABLE_FORMAT_ERROR;
199
+ goto done;
200
+ }
201
202
if (typ == BLOCK_TYPE_LOG) {
203
int block_header_skip = 4 + header_off;
@@ -203,7 +206,7 @@ int block_reader_init(struct block_reader *br, struct reftable_block *block,
206
uLongf src_len = block->len - block_header_skip;
207
/* Log blocks specify the *uncompressed* size in their header.
208
*/
206
- uint8_t *uncompressed = reftable_malloc(sz);
209
+ uncompressed = reftable_malloc(sz);
210
211
/* Copy over the block header verbatim. It's not compressed. */
212
memcpy(uncompressed, block->data, block_header_skip);
@@ -212,16 +215,19 @@ int block_reader_init(struct block_reader *br, struct reftable_block *block,
215
if (Z_OK !=
216
uncompress2(uncompressed + block_header_skip, &dst_len,
217
block->data + block_header_skip, &src_len)) {
215
- reftable_free(uncompressed);
216
- return REFTABLE_ZLIB_ERROR;
218
+ err = REFTABLE_ZLIB_ERROR;
219
+ goto done;
220
}
221
219
- if (dst_len + block_header_skip != sz)
220
- return REFTABLE_FORMAT_ERROR;
222
+ if (dst_len + block_header_skip != sz) {
223
+ err = REFTABLE_FORMAT_ERROR;
224
+ goto done;
225
+ }
226
227
/* We're done with the input data. */
228
reftable_block_done(block);
229
block->data = uncompressed;
230
+ uncompressed = NULL;
231
block->len = sz;
232
block->source = malloc_block_source();
233
full_block_size = src_len + block_header_skip;
@@ -251,7 +257,9 @@ int block_reader_init(struct block_reader *br, struct reftable_block *block,
257
br->restart_count = restart_count;
258
br->restart_bytes = restart_bytes;
259
254
- return 0;
260
+done:
261
+ reftable_free(uncompressed);
262
+ return err;
263
}
264
265
static uint32_t block_reader_restart_offset(struct block_reader *br, int i)
reftable/reader.c
+14
-9
@@ -290,28 +290,33 @@ int reader_init_block_reader(struct reftable_reader *r, struct block_reader *br,
290
291
err = reader_get_block(r, &block, next_off, guess_block_size);
292
if (err < 0)
293
- return err;
293
+ goto done;
294
295
block_size = extract_block_size(block.data, &block_typ, next_off,
296
r->version);
297
- if (block_size < 0)
298
- return block_size;
299
-
297
+ if (block_size < 0) {
298
+ err = block_size;
299
+ goto done;
300
+ }
301
if (want_typ != BLOCK_TYPE_ANY && block_typ != want_typ) {
301
- reftable_block_done(&block);
302
- return 1;
302
+ err = 1;
303
+ goto done;
304
}
305
306
if (block_size > guess_block_size) {
307
reftable_block_done(&block);
308
err = reader_get_block(r, &block, next_off, block_size);
309
if (err < 0) {
309
- return err;
310
+ goto done;
311
}
312
}
313
313
- return block_reader_init(br, &block, header_off, r->block_size,
314
- hash_size(r->hash_id));
314
+ err = block_reader_init(br, &block, header_off, r->block_size,
315
+ hash_size(r->hash_id));
316
+done:
317
+ reftable_block_done(&block);
318
+
319
+ return err;
320
}
321
322
static int table_iter_next_block(struct table_iter *dest,
reftable/readwrite_test.c
+66
@@ -254,6 +254,71 @@ static void test_log_write_read(void)
254
reader_close(&rd);
255
}
256
257
+static void test_log_zlib_corruption(void)
258
+{
259
+ struct reftable_write_options opts = {
260
+ .block_size = 256,
261
+ };
262
+ struct reftable_iterator it = { 0 };
263
+ struct reftable_reader rd = { 0 };
264
+ struct reftable_block_source source = { 0 };
265
+ struct strbuf buf = STRBUF_INIT;
266
+ struct reftable_writer *w =
267
+ reftable_new_writer(&strbuf_add_void, &buf, &opts);
268
+ const struct reftable_stats *stats = NULL;
269
+ uint8_t hash1[GIT_SHA1_RAWSZ] = { 1 };
270
+ uint8_t hash2[GIT_SHA1_RAWSZ] = { 2 };
271
+ char message[100] = { 0 };
272
+ int err, i, n;
273
+
274
+ struct reftable_log_record log = {
275
+ .refname = "refname",
276
+ .value_type = REFTABLE_LOG_UPDATE,
277
+ .value = {
278
+ .update = {
279
+ .new_hash = hash1,
280
+ .old_hash = hash2,
281
+ .name = "My Name",
282
+ .email = "myname@invalid",
283
+ .message = message,
284
+ },
285
+ },
286
+ };
287
+
288
+ for (i = 0; i < sizeof(message) - 1; i++)
289
+ message[i] = (uint8_t)(rand() % 64 + ' ');
290
+
291
+ reftable_writer_set_limits(w, 1, 1);
292
+
293
+ err = reftable_writer_add_log(w, &log);
294
+ EXPECT_ERR(err);
295
+
296
+ n = reftable_writer_close(w);
297
+ EXPECT(n == 0);
298
+
299
+ stats = writer_stats(w);
300
+ EXPECT(stats->log_stats.blocks > 0);
301
+ reftable_writer_free(w);
302
+ w = NULL;
303
+
304
+ /* corrupt the data. */
305
+ buf.buf[50] ^= 0x99;
306
+
307
+ block_source_from_strbuf(&source, &buf);
308
+
309
+ err = init_reader(&rd, &source, "file.log");
310
+ EXPECT_ERR(err);
311
+
312
+ err = reftable_reader_seek_log(&rd, &it, "refname");
313
+ EXPECT(err == REFTABLE_ZLIB_ERROR);
314
+
315
+ reftable_iterator_destroy(&it);
316
+
317
+ /* cleanup. */
318
+ strbuf_release(&buf);
319
+ reader_close(&rd);
320
+}
321
+
322
static void test_table_read_write_sequential(void)
323
{
324
char **names;
@@ -633,6 +698,7 @@ static void test_corrupt_table(void)
698
699
int readwrite_test_main(int argc, const char *argv[])
700
{
701
+ RUN_TEST(test_log_zlib_corruption);
702
RUN_TEST(test_corrupt_table);
703
RUN_TEST(test_corrupt_table_empty);
704
RUN_TEST(test_log_write_read);