CVE-2018-19198: Out-of-bounds Write (CWE-787)
value: An issue was discovered in uriparser before 0.9.0. UriQuery.c allows an out-of-bounds write via a uriComposeQuery* or uriComposeQueryEx* function because the '&' character is mishandled in certain contexts.
CWE description: The product writes data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 9.8
Published: 2018-11-12T15:29Z
Repository: https://github.com/uriparser/uriparser
Language: C++
Files changed: src/UriQuery.c, test/test.cpp
Commit date: 2018-09-23 20:54:50 +0200
Commit message: UriQuery.c: Fix out-of-bounds-write in ComposeQuery and ...Ex

Reported by Google Autofuzz team

Diff: diff --git a/src/UriQuery.c b/src/UriQuery.c
index 5fd6b68..eb22157 100644
--- a/src/UriQuery.c
+++ b/src/UriQuery.c
@@ -221,10 +221,11 @@ int URI_FUNC(ComposeQueryEngine)(URI_CHAR * dest,
 				return URI_ERROR_OUTPUT_TOO_LARGE;
 			}
 
 			/* Copy key */
 			if (firstItem == URI_TRUE) {
+				ampersandLen = 1;
 				firstItem = URI_FALSE;
 			} else {
 				write[0] = _UT('&');
 				write++;
 			}
diff --git a/test/test.cpp b/test/test.cpp
index 41e3912..dbb8adb 100644
--- a/test/test.cpp
+++ b/test/test.cpp
@@ -102,10 +102,11 @@ public:
 		TEST_ADD(UriSuite::testQueryStringEndingInEqualSign_NonBug32);
 		TEST_ADD(UriSuite::testCrash_MakeOwner_Bug20080207)
 		TEST_ADD(UriSuite::testQueryList)
 		TEST_ADD(UriSuite::testQueryListPair)
 		TEST_ADD(UriSuite::testQueryDissection_Bug3590761)
+		TEST_ADD(UriSuite::testQueryCompositionMathWrite_GoogleAutofuzz113244572)
 		TEST_ADD(UriSuite::testFreeCrash_Bug20080827)
 		TEST_ADD(UriSuite::testParseInvalid_Bug16)
 		TEST_ADD(UriSuite::testRangeComparison)
 		TEST_ADD(UriSuite::testRangeComparison_RemoveBaseUri_Issue19)
 		TEST_ADD(UriSuite::testEquals)
@@ -1747,10 +1748,41 @@ Rule                                | Example | hostSet | absPath | emptySeg
 		TEST_ASSERT(! queryList->next->next->next);
 
 		uriFreeQueryListA(queryList);
 	}
 
+	void testQueryCompositionMathWrite_GoogleAutofuzz113244572() {
+		UriQueryListA second = { .key = "\x11", .value = NULL, .next = NULL };
+		UriQueryListA first = { .key = "\x01", .value = "\x02", .next = &second };
+
+		const UriBool spaceToPlus = URI_TRUE;
+		const UriBool normalizeBreaks = URI_FALSE;  /* for factor 3 but 6 */
+
+		const int charsRequired = (3 + 1 + 3) + 1 + (3);
+
+		{
+			// Minimum space to hold everything fine
+			const char * const expected = "%01=%02" "&" "%11";
+			char dest[charsRequired + 1];
+			int charsWritten;
+			TEST_ASSERT(uriComposeQueryExA(dest, &first, sizeof(dest),
+					&charsWritten, spaceToPlus, normalizeBreaks)
+				== URI_SUCCESS);
+			TEST_ASSERT(! strcmp(dest, expected));
+			TEST_ASSERT(charsWritten == strlen(expected) + 1);
+		}
+
+		{
+			// Previous math failed to take ampersand into account
+			char dest[charsRequired + 1 - 1];
+			int charsWritten;
+			TEST_ASSERT(uriComposeQueryExA(dest, &first, sizeof(dest),
+					&charsWritten, spaceToPlus, normalizeBreaks)
+				== URI_ERROR_OUTPUT_TOO_LARGE);
+		}
+	}
+
 	void testFreeCrash_Bug20080827() {
 		char const * const sourceUri = "abc";
 		char const * const baseUri = "http://www.example.org/";
 
 		int res;

Fixed code: 				ampersandLen = 1;
		TEST_ADD(UriSuite::testQueryCompositionMathWrite_GoogleAutofuzz113244572)
	void testQueryCompositionMathWrite_GoogleAutofuzz113244572() {
		UriQueryListA second = { .key = "\x11", .value = NULL, .next = NULL };
		UriQueryListA first = { .key = "\x01", .value = "\x02", .next = &second };

		const UriBool spaceToPlus = URI_TRUE;
		const UriBool normalizeBreaks = URI_FALSE;  /* for factor 3 but 6 */

		const int charsRequired = (3 + 1 + 3) + 1 + (3);

		{
			// Minimum space to hold everything fine
			const char * const expected = "%01=%02" "&" "%11";
			char dest[charsRequired + 1];
			int charsWritten;
			TEST_ASSERT(uriComposeQueryExA(dest, &first, sizeof(dest),
					&charsWritten, spaceToPlus, normalizeBreaks)
				== URI_SUCCESS);
			TEST_ASSERT(! strcmp(dest, expected));
			TEST_ASSERT(charsWritten == strlen(expected) + 1);
		}

		{
			// Previous math failed to take ampersand into account
			char dest[charsRequired + 1 - 1];
			int charsWritten;
			TEST_ASSERT(uriComposeQueryExA(dest, &first, sizeof(dest),
					&charsWritten, spaceToPlus, normalizeBreaks)
				== URI_ERROR_OUTPUT_TOO_LARGE);
		}
	}