From: TCY16 <tom@nlnetlabs.nl>
Date: Thu, 25 Aug 2022 12:06:13 +0000 (+0200)
Subject: add better URI template checking
X-Git-Tag: release-1.18.0rc1~24^2~18^2~5
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=b642c5fe1f34cba894d8779a0a97f367cdc20d5b;p=thirdparty%2Funbound.git

add better URI template checking
---

diff --git a/sldns/str2wire.c b/sldns/str2wire.c
index 90648245b..d210e6cd0 100644
--- a/sldns/str2wire.c
+++ b/sldns/str2wire.c
@@ -1525,7 +1525,9 @@ sldns_str2wire_svcbparam_dohpath_value(const char* val,
 	uint8_t* rd, size_t* rd_len)
 {
 	size_t val_len;
-	char* open_bracket, * close_bracket, * expr_ptr;
+	char* open_bracket, * close_bracket;
+	const char* next_char;
+	uint8_t expr_found = 0;
 
 	/* RFC6570#section-2.1
 	 * "The characters outside of expressions in a URI Template string are
@@ -1541,19 +1543,36 @@ sldns_str2wire_svcbparam_dohpath_value(const char* val,
 	}
 
 	/* draft-ietf-add-svcb-dns-06#section-5.1
-	 * The URI Template MUST contain a "dns" variable
+	 * "The URI Template MUST contain a "dns" variable"
+	 * A URI Template is alowed to have multiple variables
 	 */
-	open_bracket = strchr(val, '{');
-	close_bracket = strchr(val, '}');
+	next_char = val;
+	while (next_char && *next_char != '\0') {
+		char* c;
 
-	if (!open_bracket && !close_bracket) {
-		return LDNS_WIREPARSE_ERR_SVCB_NO_DNS_VAR_IN_DOHPATH;	
-	} else {
-		expr_ptr = strstr(open_bracket+1, "?dns");
+		open_bracket = strchr(next_char, '{');
+		if (!open_bracket) {
+			return LDNS_WIREPARSE_ERR_SVCB_NO_DNS_VAR_IN_DOHPATH;
+			break;
+		}
 
-		if (!expr_ptr || !((close_bracket - expr_ptr) >= 4 ) ) {
+		close_bracket = strchr(open_bracket, '}');
+		if (!close_bracket) {
 			return LDNS_WIREPARSE_ERR_SVCB_NO_DNS_VAR_IN_DOHPATH;
+
 		}
+		for (c = open_bracket+1; (close_bracket - c) >= 4; c++) {
+			if (c[0] == '?' && c[1] == 'd' && c[2] == 'n'
+				&& c[3] == 's') {
+				expr_found++;
+			}
+		}
+
+		next_char = close_bracket+1;
+	}
+
+	if (expr_found != 1) {
+		return LDNS_WIREPARSE_ERR_SVCB_NO_DNS_VAR_IN_DOHPATH;
 	}
 
 	sldns_write_uint16(rd, SVCB_KEY_DOHPATH);
diff --git a/sldns/wire2str.c b/sldns/wire2str.c
index 75753f910..5bb13f03b 100644
--- a/sldns/wire2str.c
+++ b/sldns/wire2str.c
@@ -172,7 +172,7 @@ static sldns_lookup_table sldns_wireparse_errors_data[] = {
 	{ LDNS_WIREPARSE_ERR_SVCB_NO_DEFAULT_ALPN_VALUE,
 		"No-default-alpn should not have a value" },
 	{ LDNS_WIREPARSE_ERR_SVCB_NO_DNS_VAR_IN_DOHPATH,
-		"Dohpath must have '?dns' in the URI template variable" },
+		"Dohpath must contain a correct URI template variable which contains '?dns'" },
 	{ LDNS_WIREPARSE_ERR_SVCPARAM_BROKEN_RDATA,
 		"General SVCParam error" },
 	{ 0, NULL }
diff --git a/testdata/svcb.tdir/svcb.success-cases.zone b/testdata/svcb.tdir/svcb.success-cases.zone
index f625c6c5e..fbe1fcb5f 100644
--- a/testdata/svcb.tdir/svcb.success-cases.zone
+++ b/testdata/svcb.tdir/svcb.success-cases.zone
@@ -49,5 +49,6 @@ s09     HTTPS   0 . ( alpn="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
 ; dohpath can be (non-)quoted and MUST contain "?dns"
 
 _dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath="/dns-query{?dns}"
-_dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath=/dns-query{?dns}
+_dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath=/dns-query{?abcd}{!abcd}{?dns}
+_dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath=/dns-query{?abcdabcd?dns?defedf}
 _dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath=/dns-queryéè{?dns}
diff --git a/testdata/svcb.tdir/svcb.success-cases.zone.cmp b/testdata/svcb.tdir/svcb.success-cases.zone.cmp
index 9075cc6b5..91ea71682 100644
--- a/testdata/svcb.tdir/svcb.success-cases.zone.cmp
+++ b/testdata/svcb.tdir/svcb.success-cases.zone.cmp
@@ -9,5 +9,6 @@ s07.success-cases.	3600	IN	HTTPS	0 . ech="aGVsbG93b3JsZCE="
 s08.success-cases.	3600	IN	HTTPS	0 . key11="a" key12="a" key13="a" key14="a" key15="a" key16="a" key17="a" key18="a" key19="a" key110="a" key111="a" key112="a" key113="a" key114="a" key115="a" key116="a" key117="a" key118="a" key119="a" key120="a" key121="a" key122="a" key123="a" key124="a" key125="a" key126="a" key127="a" key128="a" key129="a" key130="a" key131="a" key132="a" key133="a" key134="a" key135="a" key136="a" key137="a" key138="a" key139="a" key140="a" key141="a" key142="a" key143="a" key144="a" key145="a" key146="a" key147="a" key148="a" key149="a" key150="a" key151="a" key152="a" key153="a" key154="a" key155="a" key156="a" key157="a" key158="a" key159="a" key160="a" key161="a" key162="a" key163="a"
 s09.success-cases.	3600	IN	HTTPS	0 . alpn="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
 _dns.doh.example.	7200	IN	SVCB	1 doh.example. alpn="h2" dohpath="/dns-query{?dns}"
-_dns.doh.example.	7200	IN	SVCB	1 doh.example. alpn="h2" dohpath="/dns-query{?dns}"
+_dns.doh.example.	7200	IN	SVCB	1 doh.example. alpn="h2" dohpath="/dns-query{?abcd}{!abcd}{?dns}"
+_dns.doh.example.	7200	IN	SVCB	1 doh.example. alpn="h2" dohpath="/dns-query{?abcdabcd?dns?defedf}"
 _dns.doh.example.	7200	IN	SVCB	1 doh.example. alpn="h2" dohpath="/dns-query\195\169\195\168{?dns}"