1 files changed, 44 insertions, 22 deletions

diff --git a/tests/t_urlencoded.c b/tests/t_urlencoded.c
index 3b3e6c2..9935c1c 100644
--- a/tests/t_urlencoded.c
+++ b/tests/t_urlencoded.c
@@ -21,7 +21,7 @@ static void case_path(void) {
   ssize_t n;
 
   test_case("path stops at the question mark");
-  n = urlencoded_parse(&p, dest, SCAN_PATH);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_PATH);
   CHECK_INT(n, 9);
   dest[n] = 0;
   CHECK_STR(dest, "/announce");
@@ -35,22 +35,22 @@ static void case_param_and_value(void) {
   ssize_t n;
 
   test_case("parameter stops at '=', value stops at '&'");
-  n = urlencoded_parse(&p, dest, SCAN_SEARCHPATH_PARAM);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_SEARCHPATH_PARAM);
   CHECK_INT(n, 9);
   dest[n] = 0;
   CHECK_STR(dest, "info_hash");
 
-  n = urlencoded_parse(&p, dest, SCAN_SEARCHPATH_VALUE);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_SEARCHPATH_VALUE);
   CHECK_INT(n, 3);
   dest[n] = 0;
   CHECK_STR(dest, "abc");
 
-  n = urlencoded_parse(&p, dest, SCAN_SEARCHPATH_PARAM);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_SEARCHPATH_PARAM);
   CHECK_INT(n, 4);
   dest[n] = 0;
   CHECK_STR(dest, "port");
 
-  n = urlencoded_parse(&p, dest, SCAN_SEARCHPATH_VALUE);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_SEARCHPATH_VALUE);
   CHECK_INT(n, 2);
   dest[n] = 0;
   CHECK_STR(dest, "80");
@@ -63,7 +63,7 @@ static void case_percent_decoding(void) {
   ssize_t n;
 
   test_case("percent escapes decode");
-  n = urlencoded_parse(&p, dest, SCAN_SEARCHPATH_VALUE);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_SEARCHPATH_VALUE);
   CHECK_INT(n, 3);
   CHECK_MEM(dest, "ABC", 3);
 }
@@ -77,7 +77,7 @@ static void case_binary_info_hash(void) {
   ssize_t       n;
 
   test_case("twenty byte binary value survives decoding");
-  n = urlencoded_parse(&p, dest, SCAN_SEARCHPATH_VALUE);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_SEARCHPATH_VALUE);
   CHECK_INT(n, 20);
   CHECK_MEM(dest, want, 20);
 }
@@ -88,7 +88,7 @@ static void case_in_place(void) {
   ssize_t n;
 
   test_case("decoding in place over the source is safe");
-  n = urlencoded_parse(&p, query, SCAN_SEARCHPATH_VALUE);
+  n = urlencoded_parse(&p, query, query + sizeof(query), SCAN_SEARCHPATH_VALUE);
   CHECK_INT(n, 2);
   CHECK_MEM(query, "AB", 2);
 }
@@ -100,7 +100,7 @@ static void case_bad_escape(void) {
   ssize_t n;
 
   test_case("a non hexadecimal escape is a parse error");
-  n = urlencoded_parse(&p, dest, SCAN_SEARCHPATH_VALUE);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_SEARCHPATH_VALUE);
   CHECK_INT(n, -1);
 }
 
@@ -111,7 +111,7 @@ static void case_truncated_escape(void) {
   ssize_t n;
 
   test_case("an escape cut short by the terminator is a parse error");
-  n = urlencoded_parse(&p, dest, SCAN_SEARCHPATH_VALUE);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_SEARCHPATH_VALUE);
   CHECK_INT(n, -1);
 }
 
@@ -123,11 +123,11 @@ static void case_terminator(void) {
   ssize_t n;
 
   test_case("a hard terminator before any output reports end of input");
-  n = urlencoded_parse(&p, dest, SCAN_SEARCHPATH_PARAM);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_SEARCHPATH_PARAM);
   CHECK_INT(n, -2);
 
   p = space;
-  n = urlencoded_parse(&p, dest, SCAN_SEARCHPATH_PARAM);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_SEARCHPATH_PARAM);
   CHECK_INT(n, -2);
 }
 
@@ -138,10 +138,10 @@ static void case_terminator_after_output(void) {
   ssize_t n;
 
   test_case("a hard terminator after output is left for the next call");
-  n = urlencoded_parse(&p, dest, SCAN_SEARCHPATH_VALUE);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_SEARCHPATH_VALUE);
   CHECK_INT(n, 3);
   CHECK_INT(*p, 0);
-  n = urlencoded_parse(&p, dest, SCAN_SEARCHPATH_VALUE);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_SEARCHPATH_VALUE);
   CHECK_INT(n, -2);
 }
 
@@ -157,14 +157,14 @@ static void case_separators_are_contextual(void) {
   ssize_t n;
 
   test_case("a question mark is literal inside a value");
-  n = urlencoded_parse(&p, dest, SCAN_SEARCHPATH_VALUE);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_SEARCHPATH_VALUE);
   CHECK_INT(n, 3);
   dest[n] = 0;
   CHECK_STR(dest, "a?b");
 
   test_case("an equals sign is literal inside a value");
   p = equals;
-  n = urlencoded_parse(&p, dest, SCAN_SEARCHPATH_VALUE);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_SEARCHPATH_VALUE);
   CHECK_INT(n, 3);
   dest[n] = 0;
   CHECK_STR(dest, "a=b");
@@ -177,7 +177,7 @@ static void case_ampersand_in_path(void) {
   ssize_t n;
 
   test_case("an ampersand in a path is a parse error");
-  n = urlencoded_parse(&p, dest, SCAN_PATH);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_PATH);
   CHECK_INT(n, -1);
 }
 
@@ -188,7 +188,7 @@ static void case_raw_high_byte(void) {
   ssize_t n;
 
   test_case("an undecoded byte above ASCII is a parse error");
-  n = urlencoded_parse(&p, dest, SCAN_SEARCHPATH_VALUE);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_SEARCHPATH_VALUE);
   CHECK_INT(n, -1);
 }
 
@@ -215,15 +215,15 @@ static void case_find_keywords(void) {
   char                    *p          = match;
 
   test_case("a known keyword returns its value");
-  CHECK_INT(urlencoded_match_keyword(keywords, &p, SCAN_SEARCHPATH_PARAM), 2);
+  CHECK_INT(urlencoded_match_keyword(keywords, &p, match + sizeof(match), SCAN_SEARCHPATH_PARAM), 2);
 
   test_case("an unknown keyword returns -3");
   p = nomatch;
-  CHECK_INT(urlencoded_match_keyword(keywords, &p, SCAN_SEARCHPATH_PARAM), -3);
+  CHECK_INT(urlencoded_match_keyword(keywords, &p, nomatch + sizeof(nomatch), SCAN_SEARCHPATH_PARAM), -3);
 
   test_case("a prefix of a keyword does not match it");
   p = prefix;
-  CHECK_INT(urlencoded_match_keyword(keywords, &p, SCAN_SEARCHPATH_PARAM), -3);
+  CHECK_INT(urlencoded_match_keyword(keywords, &p, prefix + sizeof(prefix), SCAN_SEARCHPATH_PARAM), -3);
 }
 
 /* The encoding direction. Which bytes survive unescaped is fixed by what
@@ -313,10 +313,30 @@ static void case_fmt_decoder_disagrees_on_dollar(void) {
   dest[n] = 0;
   CHECK_STR(dest, "a$b");
 
-  got = urlencoded_parse(&p, scanned, SCAN_SEARCHPATH_VALUE);
+  got = urlencoded_parse(&p, scanned, scanned + sizeof(scanned), SCAN_SEARCHPATH_VALUE);
   CHECK_INT(got, -1);
 }
 
+static void case_value_stops_at_dest_end(void) {
+  /* A value that decodes to more than the destination holds is a parse error,
+     not a write past the buffer. (ip= used to overflow ws->outbuf this way.) */
+  char  query[] = "AAAAAAAAAA&x=1";   /* ten A's then a terminator */
+  char  dest[4];
+  char *p = query;
+  test_case("a value longer than the destination returns -1");
+  CHECK_INT((int)urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_SEARCHPATH_VALUE), -1);
+}
+
+static void case_keyword_with_decoded_nul_does_not_match(void) {
+  /* A percent-encoded NUL used to make a value match a shorter keyword and
+     then index the keyword past its end. */
+  static const ot_keywords keywords[] = {{"port", 1}, {NULL, -3}};
+  char  query[] = "port%00xxxx=1";
+  char *p = query;
+  test_case("a keyword followed by a decoded NUL does not match the keyword");
+  CHECK_INT(urlencoded_match_keyword(keywords, &p, query + sizeof(query), SCAN_SEARCHPATH_PARAM), -3);
+}
+
 void test_urlencoded(void) {
   case_path();
   case_param_and_value();
@@ -332,6 +352,8 @@ void test_urlencoded(void) {
   case_raw_high_byte();
   case_skipvalue();
   case_find_keywords();
+  case_value_stops_at_dest_end();
+  case_keyword_with_decoded_nul_does_not_match();
   case_fmt_passes_safe_bytes();
   case_fmt_escapes_the_rest();
   case_fmt_hex_is_lower_case();