Skip to content

Commit 6ca0b3e

Browse files
committed
Overhaul file and buffer operation
* Add buffer limit to avoid hogging all memory on malicious inputs. * Drop use of PathIsRelative to remove dependency on shlwapi. * Always allocate size+padding. * Log an error if $INCLUDE is unsuccessful.
1 parent 35cf00f commit 6ca0b3e

File tree

7 files changed

+395
-278
lines changed

7 files changed

+395
-278
lines changed

CMakeLists.txt

-3
Original file line numberDiff line numberDiff line change
@@ -141,9 +141,6 @@ if(MINGW)
141141
endif()
142142

143143
add_library(zone STATIC)
144-
if(WIN32)
145-
target_link_libraries(zone INTERFACE shlwapi)
146-
endif()
147144

148145
generate_export_header(
149146
zone BASE_NAME ZONE EXPORT_FILE_NAME include/zone/export.h)

include/zone.h

+4-3
Original file line numberDiff line numberDiff line change
@@ -235,8 +235,8 @@ struct zone_file {
235235
// in error reports
236236
size_t span; /**< number of lines spanned by record */
237237
size_t line; /**< starting line of record */
238-
char *name;
239-
char *path;
238+
char *name; /**< filename in control directive */
239+
char *path; /**< absolute path */
240240
FILE *handle;
241241
bool grouped;
242242
bool start_of_line;
@@ -357,10 +357,11 @@ struct zone_parser {
357357
struct {
358358
size_t size;
359359
struct {
360-
size_t serial;
360+
size_t active;
361361
zone_name_buffer_t *blocks;
362362
} owner;
363363
struct {
364+
size_t active;
364365
zone_rdata_buffer_t *blocks;
365366
} rdata;
366367
} buffers;

src/generic/format.h

+12-2
Original file line numberDiff line numberDiff line change
@@ -251,8 +251,18 @@ static really_inline int32_t parse_dollar_include(
251251
file_t *file;
252252
if ((code = take_quoted_or_contiguous(parser, &include, &fields[0], token)) < 0)
253253
return code;
254-
if ((code = zone_open_file(parser, token->data, token->length, &file)) < 0)
255-
return code;
254+
255+
switch ((code = zone_open_file(parser, token->data, token->length, &file))) {
256+
case ZONE_OUT_OF_MEMORY:
257+
OUT_OF_MEMORY(parser, "Cannot open %s (%.*s), out of memory",
258+
NAME(&include), (int)token->length, token->data);
259+
case ZONE_NOT_PERMITTED:
260+
NOT_PERMITTED(parser, "Cannot open %s (%.*s), access denied",
261+
NAME(&include), (int)token->length, token->data);
262+
case ZONE_NOT_A_FILE:
263+
NOT_A_FILE(parser, "Cannot open %s (%.*s), no such file",
264+
NAME(&include), (int)token->length, token->data);
265+
}
256266

257267
name_buffer_t name;
258268
const name_buffer_t *origin = &parser->file->origin;

src/generic/parser.h

+28-12
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,8 @@ static never_inline int32_t raise_error(
227227
RAISE_ERROR((parser), ZONE_NOT_IMPLEMENTED, __VA_ARGS__)
228228
#define NOT_PERMITTED(parser, ...) \
229229
RAISE_ERROR((parser), ZONE_NOT_PERMITTED, __VA_ARGS__)
230+
#define NOT_A_FILE(parser, ...) \
231+
RAISE_ERROR((parser), ZONE_NOT_A_FILE, __VA_ARGS__)
230232

231233
// semantic errors in zone files are special as a secondary may choose
232234
// to report, but otherwise ignore them. e.g. a TTL with the MSB set. cases
@@ -243,12 +245,26 @@ static never_inline int32_t raise_error(
243245

244246
nonnull_all
245247
warn_unused_result
246-
static really_inline int32_t refill(parser_t *parser)
248+
static really_inline int32_t reindex(parser_t *parser);
249+
250+
// limit maximum size of buffer to avoid malicious inputs claiming all memory.
251+
// the maximum size of the buffer is the worst-case size of rdata, or 65535
252+
// bytes, in presentation format. comma-separated value lists as introduced
253+
// by RFC 9460 allow for double escaping. a reasonable limit is therefore
254+
// 65535 (rdata) * 4 (\DDD) * 4 (\DDD) + 64 (sufficiently large enough to
255+
// cover longest key and ancillary characters) bytes.
256+
#define MAXIMUM_WINDOW_SIZE (65535u * 4u * 4u + 64u)
257+
258+
nonnull_all
259+
warn_unused_result
260+
static int32_t refill(parser_t *parser)
247261
{
248262
// refill if possible (i.e. not if string or if file is empty)
249263
if (parser->file->end_of_file)
250264
return 0;
251265

266+
assert(parser->file->handle);
267+
252268
// move unread data to start of buffer
253269
char *data = parser->file->buffer.data + parser->file->buffer.index;
254270
// account for non-terminated character-strings
@@ -270,17 +286,21 @@ static really_inline int32_t refill(parser_t *parser)
270286

271287
// allocate extra space if required
272288
if (parser->file->buffer.length == parser->file->buffer.size) {
273-
size_t size = parser->file->buffer.size + ZONE_WINDOW_SIZE;
274-
if (!(data = realloc(data, size + 1)))
275-
OUT_OF_MEMORY(parser, "Cannot increase buffer size to %zu", size);
289+
size_t size = parser->file->buffer.size;
290+
if (parser->file->buffer.size >= MAXIMUM_WINDOW_SIZE)
291+
SYNTAX_ERROR(parser, "Impossibly large input, exceeds %zu bytes", size);
292+
size += ZONE_WINDOW_SIZE;
293+
if (!(data = realloc(data, size + 1 + ZONE_PADDING_SIZE)))
294+
OUT_OF_MEMORY(parser, "Not enough memory to allocate buffer of %zu", size);
276295
parser->file->buffer.size = size;
277296
parser->file->buffer.data = data;
278297
}
279298

280-
size_t count = fread(parser->file->buffer.data + parser->file->buffer.length,
281-
sizeof(parser->file->buffer.data[0]),
282-
parser->file->buffer.size - parser->file->buffer.length,
283-
parser->file->handle);
299+
size_t count = fread(
300+
parser->file->buffer.data + parser->file->buffer.length,
301+
sizeof(parser->file->buffer.data[0]),
302+
parser->file->buffer.size - parser->file->buffer.length,
303+
parser->file->handle);
284304

285305
if (!count && ferror(parser->file->handle))
286306
READ_ERROR(parser, "Cannot refill buffer");
@@ -292,10 +312,6 @@ static really_inline int32_t refill(parser_t *parser)
292312
return 0;
293313
}
294314

295-
nonnull_all
296-
warn_unused_result
297-
static really_inline int32_t reindex(parser_t *parser);
298-
299315
// do not invoke directly
300316
nonnull_all
301317
warn_unused_result

0 commit comments

Comments
 (0)