Skip to content

Commit a2a8578

Browse files
authored
Replace configuration file parsers with memory-safe parser (#725)
Rewrite configuration parsers using new memory safe parsing functions. After CVE-2024-25629 its obvious that we need to prioritize again on getting all the hand written parsers with direct pointer manipulation replaced. They're just not safe and hard to audit. It was yet another example of 20+yr old code having a memory safety issue just now coming to light. Though these parsers are definitely less efficient, they're written with memory safety in mind, and any performance difference is going to be meaningless for something that only happens once a while. Fix By: Brad House (@bradh352)
1 parent e4a4346 commit a2a8578

15 files changed

+842
-715
lines changed

src/lib/Makefile.inc

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ CSOURCES = ares__addrinfo2hostent.c \
1313
ares__iface_ips.c \
1414
ares__llist.c \
1515
ares__parse_into_addrinfo.c \
16-
ares__read_line.c \
1716
ares__slist.c \
1817
ares__socket.c \
1918
ares__sortaddrinfo.c \

src/lib/ares__buf.c

Lines changed: 145 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -45,47 +45,6 @@ struct ares__buf {
4545
* SIZE_MAX if not set. */
4646
};
4747

48-
ares_bool_t ares__isprint(int ch)
49-
{
50-
if (ch >= 0x20 && ch <= 0x7E) {
51-
return ARES_TRUE;
52-
}
53-
return ARES_FALSE;
54-
}
55-
56-
/* Character set allowed by hostnames. This is to include the normal
57-
* domain name character set plus:
58-
* - underscores which are used in SRV records.
59-
* - Forward slashes such as are used for classless in-addr.arpa
60-
* delegation (CNAMEs)
61-
* - Asterisks may be used for wildcard domains in CNAMEs as seen in the
62-
* real world.
63-
* While RFC 2181 section 11 does state not to do validation,
64-
* that applies to servers, not clients. Vulnerabilities have been
65-
* reported when this validation is not performed. Security is more
66-
* important than edge-case compatibility (which is probably invalid
67-
* anyhow). */
68-
ares_bool_t ares__is_hostnamech(int ch)
69-
{
70-
/* [A-Za-z0-9-*._/]
71-
* Don't use isalnum() as it is locale-specific
72-
*/
73-
if (ch >= 'A' && ch <= 'Z') {
74-
return ARES_TRUE;
75-
}
76-
if (ch >= 'a' && ch <= 'z') {
77-
return ARES_TRUE;
78-
}
79-
if (ch >= '0' && ch <= '9') {
80-
return ARES_TRUE;
81-
}
82-
if (ch == '-' || ch == '.' || ch == '_' || ch == '/' || ch == '*') {
83-
return ARES_TRUE;
84-
}
85-
86-
return ARES_FALSE;
87-
}
88-
8948
ares__buf_t *ares__buf_create(void)
9049
{
9150
ares__buf_t *buf = ares_malloc_zero(sizeof(*buf));
@@ -630,6 +589,24 @@ ares_status_t ares__buf_fetch_bytes_into_buf(ares__buf_t *buf,
630589
return ares__buf_consume(buf, len);
631590
}
632591

592+
static ares_bool_t ares__is_whitespace(unsigned char c,
593+
ares_bool_t include_linefeed)
594+
{
595+
switch (c) {
596+
case '\r':
597+
case '\t':
598+
case ' ':
599+
case '\v':
600+
case '\f':
601+
return ARES_TRUE;
602+
case '\n':
603+
return include_linefeed;
604+
default:
605+
break;
606+
}
607+
return ARES_FALSE;
608+
}
609+
633610
size_t ares__buf_consume_whitespace(ares__buf_t *buf,
634611
ares_bool_t include_linefeed)
635612
{
@@ -642,24 +619,11 @@ size_t ares__buf_consume_whitespace(ares__buf_t *buf,
642619
}
643620

644621
for (i = 0; i < remaining_len; i++) {
645-
switch (ptr[i]) {
646-
case '\r':
647-
case '\t':
648-
case ' ':
649-
case '\v':
650-
case '\f':
651-
break;
652-
case '\n':
653-
if (!include_linefeed) {
654-
goto done;
655-
}
656-
break;
657-
default:
658-
goto done;
622+
if (!ares__is_whitespace(ptr[i], include_linefeed)) {
623+
break;
659624
}
660625
}
661626

662-
done:
663627
if (i > 0) {
664628
ares__buf_consume(buf, i);
665629
}
@@ -677,20 +641,11 @@ size_t ares__buf_consume_nonwhitespace(ares__buf_t *buf)
677641
}
678642

679643
for (i = 0; i < remaining_len; i++) {
680-
switch (ptr[i]) {
681-
case '\r':
682-
case '\t':
683-
case ' ':
684-
case '\v':
685-
case '\f':
686-
case '\n':
687-
goto done;
688-
default:
689-
break;
644+
if (ares__is_whitespace(ptr[i], ARES_TRUE)) {
645+
break;
690646
}
691647
}
692648

693-
done:
694649
if (i > 0) {
695650
ares__buf_consume(buf, i);
696651
}
@@ -826,7 +781,7 @@ static ares_bool_t ares__buf_split_isduplicate(ares__llist_t *list,
826781

827782
ares_status_t ares__buf_split(ares__buf_t *buf, const unsigned char *delims,
828783
size_t delims_len, ares__buf_split_t flags,
829-
ares__llist_t **list)
784+
size_t max_sections, ares__llist_t **list)
830785
{
831786
ares_status_t status = ARES_SUCCESS;
832787
ares_bool_t first = ARES_TRUE;
@@ -842,20 +797,57 @@ ares_status_t ares__buf_split(ares__buf_t *buf, const unsigned char *delims,
842797
}
843798

844799
while (ares__buf_len(buf)) {
845-
size_t len;
800+
size_t len = 0;
801+
const unsigned char *ptr;
802+
803+
if (first) {
804+
/* No delimiter yet, just tag the start */
805+
ares__buf_tag(buf);
806+
} else {
807+
if (flags & ARES_BUF_SPLIT_DONT_CONSUME_DELIMS) {
808+
/* tag then eat delimiter so its first byte in buffer */
809+
ares__buf_tag(buf);
810+
ares__buf_consume(buf, 1);
811+
} else {
812+
/* throw away delimiter */
813+
ares__buf_consume(buf, 1);
814+
ares__buf_tag(buf);
815+
}
816+
}
817+
818+
if (max_sections && ares__llist_len(*list) >= max_sections - 1) {
819+
ares__buf_consume(buf, ares__buf_len(buf));
820+
} else {
821+
ares__buf_consume_until_charset(buf, delims, delims_len, ARES_FALSE);
822+
}
846823

847-
ares__buf_tag(buf);
824+
ptr = ares__buf_tag_fetch(buf, &len);
848825

849-
len = ares__buf_consume_until_charset(buf, delims, delims_len, ARES_FALSE);
826+
/* Shouldn't be possible */
827+
if (ptr == NULL) {
828+
status = ARES_EFORMERR;
829+
goto done;
830+
}
831+
832+
if (flags & ARES_BUF_SPLIT_LTRIM) {
833+
size_t i;
834+
for (i = 0; i < len; i++) {
835+
if (!ares__is_whitespace(ptr[i], ARES_TRUE)) {
836+
break;
837+
}
838+
}
839+
ptr += i;
840+
len -= i;
841+
}
850842

851-
/* Don't treat a delimiter as part of the length */
852-
if (!first && len && flags & ARES_BUF_SPLIT_DONT_CONSUME_DELIMS) {
853-
len--;
843+
if (flags & ARES_BUF_SPLIT_RTRIM) {
844+
while (len && ares__is_whitespace(ptr[len - 1], ARES_TRUE)) {
845+
len--;
846+
}
854847
}
855848

856849
if (len != 0 || flags & ARES_BUF_SPLIT_ALLOW_BLANK) {
857-
const unsigned char *ptr = ares__buf_tag_fetch(buf, &len);
858-
ares__buf_t *data;
850+
ares__buf_t *data;
859851

860852
if (!(flags & ARES_BUF_SPLIT_NO_DUPLICATES) ||
861853
!ares__buf_split_isduplicate(*list, ptr, len, flags)) {
@@ -880,12 +872,6 @@ ares_status_t ares__buf_split(ares__buf_t *buf, const unsigned char *delims,
880872
}
881873
}
882874

883-
if (!(flags & ARES_BUF_SPLIT_DONT_CONSUME_DELIMS) &&
884-
ares__buf_len(buf) != 0) {
885-
/* Consume delimiter */
886-
ares__buf_consume(buf, 1);
887-
}
888-
889875
first = ARES_FALSE;
890876
}
891877

@@ -1150,3 +1136,80 @@ ares_status_t ares__buf_hexdump(ares__buf_t *buf, const unsigned char *data,
11501136

11511137
return ARES_SUCCESS;
11521138
}
1139+
1140+
ares_status_t ares__buf_load_file(const char *filename, ares__buf_t *buf)
1141+
{
1142+
FILE *fp = NULL;
1143+
unsigned char *ptr = NULL;
1144+
size_t len = 0;
1145+
size_t ptr_len = 0;
1146+
long ftell_len = 0;
1147+
ares_status_t status;
1148+
1149+
if (filename == NULL || buf == NULL) {
1150+
return ARES_EFORMERR;
1151+
}
1152+
1153+
fp = fopen(filename, "rb");
1154+
if (fp == NULL) {
1155+
int error = ERRNO;
1156+
switch (error) {
1157+
case ENOENT:
1158+
case ESRCH:
1159+
status = ARES_ENOTFOUND;
1160+
goto done;
1161+
default:
1162+
DEBUGF(fprintf(stderr, "fopen() failed with error: %d %s\n", error,
1163+
strerror(error)));
1164+
DEBUGF(fprintf(stderr, "Error opening file: %s\n", filename));
1165+
status = ARES_EFILE;
1166+
goto done;
1167+
}
1168+
}
1169+
1170+
/* Get length portably, fstat() is POSIX, not C */
1171+
if (fseek(fp, 0, SEEK_END) != 0) {
1172+
status = ARES_EFILE;
1173+
goto done;
1174+
}
1175+
1176+
ftell_len = ftell(fp);
1177+
if (ftell_len < 0) {
1178+
status = ARES_EFILE;
1179+
goto done;
1180+
}
1181+
len = (size_t)ftell_len;
1182+
1183+
if (fseek(fp, 0, SEEK_SET) != 0) {
1184+
status = ARES_EFILE;
1185+
goto done;
1186+
}
1187+
1188+
if (len == 0) {
1189+
status = ARES_SUCCESS;
1190+
goto done;
1191+
}
1192+
1193+
/* Read entire data into buffer */
1194+
ptr_len = len;
1195+
ptr = ares__buf_append_start(buf, &ptr_len);
1196+
if (ptr == NULL) {
1197+
status = ARES_ENOMEM;
1198+
goto done;
1199+
}
1200+
1201+
ptr_len = fread(ptr, 1, len, fp);
1202+
if (ptr_len != len) {
1203+
status = ARES_EFILE;
1204+
goto done;
1205+
}
1206+
1207+
ares__buf_append_finish(buf, len);
1208+
status = ARES_SUCCESS;
1209+
1210+
done:
1211+
if (fp != NULL) {
1212+
fclose(fp);
1213+
}
1214+
return status;
1215+
}

src/lib/ares__buf.h

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,8 @@ size_t ares__buf_consume_whitespace(ares__buf_t *buf,
373373
size_t ares__buf_consume_nonwhitespace(ares__buf_t *buf);
374374

375375

376-
/*! Consume until a character in the character set provided is reached
376+
/*! Consume until a character in the character set provided is reached. Does
377+
* not include the character from the charset at the end.
377378
*
378379
* \param[in] buf Initialized buffer object
379380
* \param[in] charset character set
@@ -414,7 +415,9 @@ typedef enum {
414415
/*! No flags */
415416
ARES_BUF_SPLIT_NONE = 0,
416417
/*! The delimiter will be the first character in the buffer, except the
417-
* first buffer since the start doesn't have a delimiter
418+
* first buffer since the start doesn't have a delimiter. This option is
419+
* incompatible with ARES_BUF_SPLIT_LTRIM since the delimiter is always
420+
* the first character.
418421
*/
419422
ARES_BUF_SPLIT_DONT_CONSUME_DELIMS = 1 << 0,
420423
/*! Allow blank sections, by default blank sections are not emitted. If using
@@ -424,7 +427,13 @@ typedef enum {
424427
/*! Remove duplicate entries */
425428
ARES_BUF_SPLIT_NO_DUPLICATES = 1 << 2,
426429
/*! Perform case-insensitive matching when comparing values */
427-
ARES_BUF_SPLIT_CASE_INSENSITIVE = 1 << 3
430+
ARES_BUF_SPLIT_CASE_INSENSITIVE = 1 << 3,
431+
/*! Trim leading whitespace from buffer */
432+
ARES_BUF_SPLIT_LTRIM = 1 << 4,
433+
/*! Trim trailing whitespace from buffer */
434+
ARES_BUF_SPLIT_RTRIM = 1 << 5,
435+
/*! Trim leading and trailing whitespace from buffer */
436+
ARES_BUF_SPLIT_TRIM = (ARES_BUF_SPLIT_LTRIM | ARES_BUF_SPLIT_RTRIM)
428437
} ares__buf_split_t;
429438

430439
/*! Split the provided buffer into multiple sub-buffers stored in the variable
@@ -435,6 +444,12 @@ typedef enum {
435444
* \param[in] delims Possible delimiters
436445
* \param[in] delims_len Length of possible delimiters
437446
* \param[in] flags One more more flags
447+
* \param[in] max_sections Maximum number of sections. Use 0 for
448+
* unlimited. Useful for splitting key/value
449+
* pairs where the delimiter may be a valid
450+
* character in the value. A value of 1 would
451+
* have little usefulness and would effectively
452+
* ignore the delimiter itself.
438453
* \param[out] list Result. Depending on flags, this may be a
439454
* valid list with no elements. Use
440455
* ares__llist_destroy() to free the memory which
@@ -444,7 +459,7 @@ typedef enum {
444459
*/
445460
ares_status_t ares__buf_split(ares__buf_t *buf, const unsigned char *delims,
446461
size_t delims_len, ares__buf_split_t flags,
447-
ares__llist_t **list);
462+
size_t max_sections, ares__llist_t **list);
448463

449464

450465
/*! Check the unprocessed buffer to see if it begins with the sequence of
@@ -567,6 +582,18 @@ ares_status_t ares__buf_parse_dns_str(ares__buf_t *buf, size_t remaining_len,
567582
ares_status_t ares__buf_parse_dns_binstr(ares__buf_t *buf, size_t remaining_len,
568583
unsigned char **bin, size_t *bin_len,
569584
ares_bool_t allow_multiple);
585+
586+
/*! Load data from specified file path into provided buffer. The entire file
587+
* is loaded into memory.
588+
*
589+
* \param[in] filename complete path to file
590+
* \param[in,out] buf Initialized (non-const) buffer object to load data
591+
* into
592+
* \return ARES_ENOTFOUND if file not found, ARES_EFILE if issues reading
593+
* file, ARES_ENOMEM if out of memory, ARES_SUCCESS on success.
594+
*/
595+
ares_status_t ares__buf_load_file(const char *filename, ares__buf_t *buf);
596+
570597
/*! @} */
571598

572599
#endif /* __ARES__BUF_H */

0 commit comments

Comments
 (0)