Skip to content

Commit 31a3ad7

Browse files
committed
Improve buffer handling in data_buf.c
* Potential overflow in databuf_append() when DATABUF_FLAG_PRESERVE_HEAD is set and data has been advanced: failure of databuf_shift_data_to_beginning() is ignored, so appending can write past the reallocated buffer. Inefficient reallocations in databuf_append(): the buffer grows exactly to new_size, causing repeated reallocations on incremental feeds. Growing exponentially could improve performance. Reworked the data buffer expansion logic to double the allocation size when growing and account for existing offsets.
1 parent 50ef75c commit 31a3ad7

File tree

1 file changed

+39
-34
lines changed

1 file changed

+39
-34
lines changed

auparse/data_buf.c

Lines changed: 39 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/* data_buf.c --
2-
* Copyright 2007,2011 Red Hat Inc., Durham, North Carolina.
2+
* Copyright 2007,2011 Red Hat Inc
33
* All Rights Reserved.
44
*
55
* This library is free software; you can redistribute it and/or
@@ -193,50 +193,55 @@ void databuf_free(DataBuf *db)
193193

194194
int databuf_append(DataBuf *db, const char *src, size_t src_size)
195195
{
196-
size_t new_size;
196+
size_t new_len, required_size;
197197

198-
DATABUF_VALIDATE(db);
198+
DATABUF_VALIDATE(db);
199199

200-
if (src == NULL || src_size == 0) return 0;
200+
if (src == NULL || src_size == 0)
201+
return 0;
201202

202-
new_size = db->len+src_size;
203+
new_len = db->len + src_size;
203204

204205
#ifdef DEBUG
205-
if (debug) databuf_print(db, 1, "databuf_append() size=%zd", src_size);
206+
if (debug)
207+
databuf_print(db, 1, "databuf_append() size=%zd", src_size);
206208
#endif
207-
if ((new_size > db->alloc_size) ||
208-
((db->flags & DATABUF_FLAG_PRESERVE_HEAD) &&
209-
!databuf_tail_available(db, src_size))) {
210-
/* not enough room, we must realloc */
211-
void *new_alloc;
212-
213-
databuf_shift_data_to_beginning(db);
214-
if ((new_alloc = realloc(db->alloc_ptr, new_size))) {
215-
db->alloc_ptr = new_alloc;
216-
db->alloc_size = new_size;
217-
} else {
218-
return -1; /* realloc failed */
219-
}
220-
} else {
221-
/* we can fit within current allocation, but can we append? */
222-
if (!databuf_tail_available(db, src_size)) {
223-
/* we can't append in place, must create room at tail by shifting
224-
data forward to the beginning of the allocation block */
225-
databuf_shift_data_to_beginning(db);
209+
210+
/* If we can shift data to the beginning do so before any size checks */
211+
if (!(db->flags & DATABUF_FLAG_PRESERVE_HEAD) &&
212+
!databuf_tail_available(db, src_size))
213+
databuf_shift_data_to_beginning(db);
214+
215+
required_size = db->offset + new_len;
216+
if (required_size > db->alloc_size) {
217+
void *new_alloc;
218+
size_t new_alloc_size = db->alloc_size ?
219+
db->alloc_size * 2 : required_size;
220+
221+
if (new_alloc_size < required_size)
222+
new_alloc_size = required_size;
223+
224+
new_alloc = realloc(db->alloc_ptr, new_alloc_size);
225+
if (!new_alloc)
226+
return -1; /* realloc failed */
227+
228+
db->alloc_ptr = new_alloc;
229+
db->alloc_size = new_alloc_size;
226230
}
227-
}
228231
#ifdef DEBUG
229-
if (debug) databuf_print(db, 1, "databuf_append() about to memmove()");
232+
if (debug)
233+
databuf_print(db, 1, "databuf_append() about to memmove()");
230234
#endif
231-
/* pointers all set up and room available, move the data and update */
232-
memmove(databuf_end(db), src, src_size);
233-
db->len = new_size;
234-
db->max_len = MAX(db->max_len, new_size);
235+
/* pointers all set up and room available, move the data and update */
236+
memmove(databuf_end(db), src, src_size);
237+
db->len = new_len;
238+
db->max_len = MAX(db->max_len, new_len);
235239
#ifdef DEBUG
236-
if (debug) databuf_print(db, 1, "databuf_append() conclusion");
240+
if (debug)
241+
databuf_print(db, 1, "databuf_append() conclusion");
237242
#endif
238-
DATABUF_VALIDATE(db);
239-
return 1;
243+
DATABUF_VALIDATE(db);
244+
return 1;
240245
}
241246

242247
int databuf_replace(DataBuf *db, const char *src, size_t src_size)

0 commit comments

Comments
 (0)