Skip to content

Commit 6e7dd7f

Browse files
committed
Catch unterminated strings
1 parent ecfb3a1 commit 6e7dd7f

File tree

7 files changed

+126
-24
lines changed

7 files changed

+126
-24
lines changed

include/zone.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ struct zone_file {
240240
FILE *handle;
241241
bool grouped;
242242
bool start_of_line;
243-
enum { ZONE_HAVE_DATA, ZONE_READ_ALL_DATA, ZONE_NO_MORE_DATA } end_of_file;
243+
uint8_t end_of_file;
244244
struct {
245245
size_t index, length, size;
246246
char *data;

src/fallback/scanner.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -153,10 +153,10 @@ static really_inline int32_t reindex(parser_t *parser)
153153
if (parser->file->end_of_file) {
154154
assert(left < ZONE_BLOCK_SIZE);
155155
if (!left) {
156-
parser->file->end_of_file = ZONE_NO_MORE_DATA;
156+
parser->file->end_of_file = NO_MORE_DATA;
157157
} else if (((uintptr_t)tape_limit - (uintptr_t)tape) >= left) {
158158
scan(parser, data, data + left);
159-
parser->file->end_of_file = ZONE_NO_MORE_DATA;
159+
parser->file->end_of_file = NO_MORE_DATA;
160160
parser->file->buffer.index += left;
161161
parser->file->state.follows_contiguous = 0;
162162
}

src/generic/format.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,7 @@ static inline int32_t parse(parser_t *parser)
376376

377377
code = parse_rr(parser, &token);
378378
} else if (is_end_of_file(&token)) {
379-
if (parser->file->end_of_file == ZONE_NO_MORE_DATA)
379+
if (parser->file->end_of_file == NO_MORE_DATA)
380380
break;
381381
} else if (is_line_feed(&token)) {
382382
assert(token.code == LINE_FEED);

src/generic/parser.h

+24-11
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,10 @@ static const char line_feed[ZONE_BLOCK_SIZE] = { '\n', '\0' };
175175
// special constant used as data on errors
176176
static const char end_of_file[ZONE_BLOCK_SIZE] = { '\0' };
177177

178+
#define READ_ALL_DATA (1)
179+
#define NO_MORE_DATA (2)
180+
#define MISSING_QUOTE (3)
181+
178182
extern int32_t zone_open_file(
179183
parser_t *, const char *path, size_t length, zone_file_t **);
180184

@@ -213,6 +217,8 @@ static never_inline int32_t raise_error(
213217
return raise_error((parser), (code), __VA_ARGS__); \
214218
} while (0)
215219

220+
#define SYNTAX_ERROR(parser, ...) \
221+
RAISE_ERROR((parser), ZONE_SYNTAX_ERROR, __VA_ARGS__)
216222
#define OUT_OF_MEMORY(parser, ...) \
217223
RAISE_ERROR((parser), ZONE_OUT_OF_MEMORY, __VA_ARGS__)
218224
#define READ_ERROR(parser, ...) \
@@ -240,14 +246,14 @@ warn_unused_result
240246
static really_inline int32_t refill(parser_t *parser)
241247
{
242248
// refill if possible (i.e. not if string or if file is empty)
243-
if (parser->file->end_of_file != ZONE_HAVE_DATA)
249+
if (parser->file->end_of_file)
244250
return 0;
245251

246252
// move unread data to start of buffer
247253
char *data = parser->file->buffer.data + parser->file->buffer.index;
248254
// account for non-terminated character-strings
249-
if (*parser->file->fields.head)
250-
data = (char *)*parser->file->fields.head;
255+
if (*parser->file->fields.head[0] != '\0')
256+
data = (char *)parser->file->fields.head[0];
251257

252258
*parser->file->fields.head = parser->file->buffer.data;
253259
// account for unread data left in buffer
@@ -305,20 +311,27 @@ static really_inline int32_t advance(parser_t *parser)
305311
parser->file->fields.tape[0] = parser->file->fields.tail[1];
306312
parser->file->fields.head = parser->file->fields.tape;
307313
parser->file->fields.tail =
308-
parser->file->fields.tape + (!!parser->file->fields.tape[0]);
314+
parser->file->fields.tape + (*parser->file->fields.tape[0] != '\0');
309315
// reset delimiters
310316
parser->file->delimiters.head = parser->file->delimiters.tape;
311317
parser->file->delimiters.tail = parser->file->delimiters.tape;
312318

319+
// delayed syntax error
320+
if (parser->file->end_of_file == MISSING_QUOTE)
321+
SYNTAX_ERROR(parser, "Missing closing quote");
313322
if ((code = refill(parser)) < 0)
314323
return code;
315324

316325
if (reindex(parser)) {
317326
// save non-terminated token
318327
parser->file->fields.tail[0] = parser->file->fields.tail[-1];
319328
parser->file->fields.tail--;
329+
// delay syntax error so correct line number is available
330+
if (parser->file->end_of_file == NO_MORE_DATA &&
331+
*parser->file->fields.tail[1] == '"')
332+
parser->file->end_of_file = MISSING_QUOTE;
320333
} else {
321-
parser->file->fields.tail[1] = NULL;
334+
parser->file->fields.tail[1] = end_of_file;
322335
}
323336

324337
// FIXME: if tail is still equal to tape, refill immediately?!
@@ -331,7 +344,6 @@ static really_inline int32_t advance(parser_t *parser)
331344
// start-of-line must be false if start of tape is not start of buffer
332345
if (*parser->file->fields.head != parser->file->buffer.data)
333346
parser->file->start_of_line = false;
334-
//parser->file->start_of_line = start_of_line;
335347
return 0;
336348
}
337349

@@ -378,6 +390,7 @@ static really_inline bool is_end_of_file(const token_t *token)
378390
}
379391

380392

393+
#undef SYNTAX_ERROR
381394
#define SYNTAX_ERROR(parser, token, ...) \
382395
do { \
383396
zone_log((parser), ZONE_ERROR, __VA_ARGS__); \
@@ -425,7 +438,7 @@ static never_inline void maybe_take(parser_t *parser, token_t *token)
425438
return;
426439
} else if (token->code == END_OF_FILE) {
427440
int32_t code;
428-
if (parser->file->end_of_file == ZONE_NO_MORE_DATA) {
441+
if (parser->file->end_of_file == NO_MORE_DATA) {
429442
if (parser->file->grouped)
430443
SYNTAX_ERROR(parser, token, "Missing closing brace");
431444
token->data = end_of_file;
@@ -565,7 +578,7 @@ static never_inline int32_t maybe_take_contiguous(
565578
parser->file->delimiters.head++;
566579
return 0;
567580
} else if (token->code == END_OF_FILE) {
568-
if (parser->file->end_of_file == ZONE_NO_MORE_DATA)
581+
if (parser->file->end_of_file == NO_MORE_DATA)
569582
SYNTAX_ERROR(parser, token, "Missing %s in %s", NAME(field), NAME(type));
570583
if ((code = advance(parser)) < 0)
571584
ERROR(parser, token, code);
@@ -666,7 +679,7 @@ static never_inline int32_t maybe_take_quoted(
666679
parser->file->delimiters.head++;
667680
return 0;
668681
} else if (token->code == END_OF_FILE) {
669-
if (parser->file->end_of_file == ZONE_NO_MORE_DATA)
682+
if (parser->file->end_of_file == NO_MORE_DATA)
670683
SYNTAX_ERROR(parser, token, "Missing %s in %s", NAME(field), NAME(type));
671684
if ((code = advance(parser)) < 0)
672685
ERROR(parser, token, code);
@@ -773,7 +786,7 @@ static never_inline int32_t maybe_take_contiguous_or_quoted(
773786
parser->file->delimiters.head++;
774787
return 0;
775788
} else if (token->code == END_OF_FILE) {
776-
if (parser->file->end_of_file == ZONE_NO_MORE_DATA)
789+
if (parser->file->end_of_file == NO_MORE_DATA)
777790
SYNTAX_ERROR(parser, token, "Missing %s in %s", NAME(field), NAME(type));
778791
if ((code = advance(parser)) < 0)
779792
ERROR(parser, token, code);
@@ -894,7 +907,7 @@ static never_inline int32_t maybe_take_delimiter(
894907
return 0;
895908
}
896909
} else if (token->code == END_OF_FILE) {
897-
if (parser->file->end_of_file == ZONE_NO_MORE_DATA) {
910+
if (parser->file->end_of_file == NO_MORE_DATA) {
898911
if (parser->file->grouped)
899912
SYNTAX_ERROR(parser, token, "Missing closing brace");
900913
token->data = end_of_file;

src/generic/scanner.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ static really_inline int32_t reindex(parser_t *parser)
312312
if (parser->file->end_of_file) {
313313
assert(left < ZONE_BLOCK_SIZE);
314314
if (!left) {
315-
parser->file->end_of_file = ZONE_NO_MORE_DATA;
315+
parser->file->end_of_file = NO_MORE_DATA;
316316
} else if (((uintptr_t)tape_limit - (uintptr_t)tape) >= left) {
317317
// input is required to be padded, but may contain garbage
318318
uint8_t buffer[ZONE_BLOCK_SIZE] = { 0 };
@@ -322,7 +322,7 @@ static really_inline int32_t reindex(parser_t *parser)
322322
scan(parser, &block);
323323
block.contiguous &= ~clear;
324324
write_indexes(parser, &block, clear);
325-
parser->file->end_of_file = ZONE_NO_MORE_DATA;
325+
parser->file->end_of_file = NO_MORE_DATA;
326326
parser->file->buffer.index += left;
327327
}
328328
}

src/zone.c

+6-6
Original file line numberDiff line numberDiff line change
@@ -222,9 +222,8 @@ static int32_t open_file(
222222
file->buffer.length = 0;
223223
file->buffer.index = 0;
224224
file->start_of_line = true;
225-
file->end_of_file = ZONE_HAVE_DATA;
226-
file->fields.tape[0] = file->buffer.data;
227-
file->fields.tape[1] = NULL;
225+
file->end_of_file = 0;
226+
file->fields.tape[0] = file->fields.tape[1] = file->buffer.data;
228227
file->fields.head = file->fields.tape;
229228
file->fields.tail = file->fields.tape;
230229
file->lines.tape[0] = 0;
@@ -376,6 +375,8 @@ int32_t zone_parse_string(
376375
zone_file_t *file;
377376
int32_t result;
378377

378+
if (!length || string[length] != '\0')
379+
return ZONE_BAD_PARAMETER;
379380
if ((result = check_options(options)) < 0)
380381
return result;
381382

@@ -393,9 +394,8 @@ int32_t zone_parse_string(
393394
file->buffer.size = length;
394395
file->buffer.data = (char *)string;
395396
file->start_of_line = true;
396-
file->end_of_file = ZONE_READ_ALL_DATA;
397-
file->fields.tape[0] = "\0";
398-
file->fields.tape[1] = NULL;
397+
file->end_of_file = 1;
398+
file->fields.tape[0] = file->fields.tape[1] = &string[length];
399399
file->fields.head = file->fields.tape;
400400
file->fields.tail = file->fields.tape;
401401
file->lines.tape[0] = 0;

tests/syntax.c

+90-1
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,14 @@
77
*
88
*/
99
#include <assert.h>
10+
#include <limits.h>
1011
#include <stdarg.h>
1112
#include <setjmp.h>
1213
#include <string.h>
1314
#include <cmocka.h>
15+
#if !_WIN32
16+
#include <unistd.h>
17+
#endif
1418

1519
#include "zone.h"
1620

@@ -72,7 +76,7 @@ void newlines(void **state)
7276
// >> do the same thing for contiguous
7377
#endif
7478
static const char grouped_lf_text[] =
75-
PAD("1. TXT (\nfoo\nbar\n)\"\n2. TXT \"foobar\"");
79+
PAD("1. TXT (\nfoo\nbar\n)\n2. TXT \"foobar\"");
7680
static const char plain_lf_text[] =
7781
PAD("1. TXT \"foo bar\"\n2. TXT \"foo baz\"");
7882
static const char control_lf_text[] =
@@ -108,6 +112,7 @@ void newlines(void **state)
108112
options.default_ttl = 3600;
109113
options.default_class = ZONE_IN;
110114

115+
fprintf(stderr, "INPUT: \"%s\"\n", tests[i].input);
111116
result = zone_parse_string(
112117
&parser, &options, &buffers, tests[i].input, strlen(tests[i].input), (void*)&tests[i]);
113118
assert_int_equal(result, ZONE_SUCCESS);
@@ -471,3 +476,87 @@ void ttls(void **state)
471476
assert_int_equal(code, tests[i].code);
472477
}
473478
}
479+
480+
static int32_t dummy_callback(
481+
zone_parser_t *parser,
482+
const zone_name_t *owner,
483+
uint16_t type,
484+
uint16_t class,
485+
uint32_t ttl,
486+
uint16_t rdlength,
487+
const uint8_t *rdata,
488+
void *user_data)
489+
{
490+
(void)parser;
491+
(void)owner;
492+
(void)type;
493+
(void)class;
494+
(void)ttl;
495+
(void)rdlength;
496+
(void)rdata;
497+
(void)user_data;
498+
return 0;
499+
}
500+
501+
static int32_t parse_text(const char *text)
502+
{
503+
zone_parser_t parser;
504+
zone_name_buffer_t name;
505+
zone_rdata_buffer_t rdata;
506+
zone_buffers_t buffers = { 1, &name, &rdata };
507+
zone_options_t options = { 0 };
508+
const uint8_t origin[] = { 0 };
509+
510+
options.accept.callback = &dummy_callback;
511+
options.origin.octets = origin;
512+
options.origin.length = sizeof(origin);
513+
options.default_ttl = 3600;
514+
options.default_class = 1;
515+
516+
fprintf(stderr, "INPUT: '%s'\n", text);
517+
return zone_parse_string(&parser, &options, &buffers, text, strlen(text), NULL);
518+
}
519+
520+
static char *generate_include(const char *text)
521+
{
522+
char *path = tempnam(NULL, "zone");
523+
if (path) {
524+
FILE *handle = fopen(path, "wbx");
525+
if (handle) {
526+
int result = fputs(text, handle);
527+
(void)fclose(handle);
528+
if (result != EOF)
529+
return path;
530+
}
531+
free(path);
532+
}
533+
return NULL;
534+
}
535+
536+
/*!cmocka */
537+
void quote_no_unquote(void **state)
538+
{
539+
(void)state;
540+
541+
int32_t code;
542+
static const char *no_unquote = PAD("foo. TXT \"unterminated string");
543+
544+
// verify unterminated strings are caught
545+
code = parse_text(no_unquote);
546+
assert_int_equal(code, ZONE_SYNTAX_ERROR);
547+
548+
// verify unterminated strings are caught in included file
549+
char *path = generate_include(no_unquote);
550+
assert_non_null(path);
551+
char dummy[16];
552+
int length = snprintf(dummy, sizeof(dummy), "$INCLUDE \"%s\"\n", path);
553+
assert_true(length > 0 && length < INT_MAX - ZONE_PADDING_SIZE);
554+
char *include = malloc((size_t)length + 1 + ZONE_PADDING_SIZE);
555+
assert_non_null(include);
556+
(void)snprintf(include, (size_t)length + 1, "$INCLUDE \"%s\"\n", path);
557+
code = parse_text(include);
558+
assert_int_equal(code, ZONE_SYNTAX_ERROR);
559+
free(include);
560+
unlink(path);
561+
free(path);
562+
}

0 commit comments

Comments
 (0)