From b51c9eb7975ad883fc6380347b9be56d4976e730 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Mon, 17 Jun 2024 23:16:28 +1000 Subject: [PATCH] Properly reject zero length ALPN in commatxt_fromtext ALPN are defined as 1*255OCTET in RFC 9460. commatxt_fromtext was not rejecting invalid inputs produces by missing a level of escaping which where later caught be dns_rdata_fromwire on reception. These inputs should have been rejected svcb in svcb 1 1.svcb alpn=\,abc svcb1 in svcb 1 1.svcb alpn=a\,\,abc and generated 00 03 61 62 63 and 01 61 00 02 61 62 63 respectively. The correct inputs to include commas in the alpn requires double escaping. svcb in svcb 1 1.svcb alpn=\\,abc svcb1 in svcb 1 1.svcb alpn=a\\,\\,abc and generate 04 2C 61 62 63 and 06 61 2C 2C 61 62 63 respectively. --- lib/dns/rdata.c | 17 +++++++++++------ tests/dns/rdata_test.c | 4 ++++ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/lib/dns/rdata.c b/lib/dns/rdata.c index 4570029152..4f81fbfb3a 100644 --- a/lib/dns/rdata.c +++ b/lib/dns/rdata.c @@ -1668,21 +1668,26 @@ commatxt_fromtext(isc_textregion_t *source, bool comma, isc_buffer_t *target) { if (comma) { /* - * Disallow empty ALPN at start (",h1") or in the - * middle ("h1,,h2"). + * Disallow empty ALPN at start (",h1" or "\,h1") or + * in the middle ("h1,,h2" or "h1\,\,h2"). */ - if (s == source->base || (seen_comma && s == source->base + 1)) - { + if ((t - tregion.base - 1) == 0) { return (DNS_R_SYNTAX); } - isc_textregion_consume(source, s - source->base); + /* - * Disallow empty ALPN at end ("h1,"). + * Consume this ALPN and possible ending comma. + */ + isc_textregion_consume(source, s - source->base); + + /* + * Disallow empty ALPN at end ("h1," or "h1\,"). */ if (seen_comma && source->length == 0) { return (DNS_R_SYNTAX); } } + *tregion.base = (unsigned char)(t - tregion.base - 1); isc_buffer_add(target, *tregion.base + 1); return (ISC_R_SUCCESS); diff --git a/tests/dns/rdata_test.c b/tests/dns/rdata_test.c index f5746279ab..2b675b1864 100644 --- a/tests/dns/rdata_test.c +++ b/tests/dns/rdata_test.c @@ -2599,6 +2599,10 @@ ISC_RUN_TEST_IMPL(https_svcb) { TEXT_INVALID("2 svc.example.net. alpn=,h1"), TEXT_INVALID("2 svc.example.net. alpn=h1,"), TEXT_INVALID("2 svc.example.net. alpn=h1,,h2"), + /* empty alpn-id sub fields - RFC 1035 escaped commas */ + TEXT_INVALID("2 svc.example.net. alpn=\\,abc"), + TEXT_INVALID("2 svc.example.net. alpn=abc\\,"), + TEXT_INVALID("2 svc.example.net. alpn=a\\,\\,abc"), /* mandatory */ TEXT_VALID_LOOP(2, "2 svc.example.net. mandatory=alpn " "alpn=\"h2\""),