Fixed bug #1656547: segfault in urlUtils::decode() if the string ends with '%'.

2 files changed, 28 insertions, 19 deletions

diff --git a/src/utility/urlUtils.cpp b/src/utility/urlUtils.cpp
index 52405c07..90dc7887 100644
--- a/src/utility/urlUtils.cpp
+++ b/src/utility/urlUtils.cpp
@@ -79,14 +79,16 @@ const string urlUtils::decode(const string& s)
 		{
 		case '%':
 		{
-			const char_t p = (++it != s.end() ? *it : 0);
-			const char_t q = (++it != s.end() ? *it : 0);
+			++it;  // skip '%'
+
+			const char_t p = (it != s.end() ? *(it++) : 0);
+			const char_t q = (it != s.end() ? *(it++) : 0);
 
 			unsigned char r = 0;
 
 			switch (p)
 			{
-			case 0: r = '?'; break;
+			case 0: r = '%'; break;
 			case 'a': case 'A': r = 10; break;
 			case 'b': case 'B': r = 11; break;
 			case 'c': case 'C': r = 12; break;
@@ -96,25 +98,23 @@ const string urlUtils::decode(const string& s)
 			default: r = p - '0'; break;
 			}
 
-			r *= 16;
-
-			switch (q)
+			if (q != 0)
 			{
-			case 0: r = '?'; break;
-			case 'a': case 'A': r += 10; break;
-			case 'b': case 'B': r += 11; break;
-			case 'c': case 'C': r += 12; break;
-			case 'd': case 'D': r += 13; break;
-			case 'e': case 'E': r += 14; break;
-			case 'f': case 'F': r += 15; break;
-			default: r += q - '0'; break;
+				r *= 16;
+
+				switch (q)
+				{
+				case 'a': case 'A': r += 10; break;
+				case 'b': case 'B': r += 11; break;
+				case 'c': case 'C': r += 12; break;
+				case 'd': case 'D': r += 13; break;
+				case 'e': case 'E': r += 14; break;
+				case 'f': case 'F': r += 15; break;
+				default: r += q - '0'; break;
+				}
 			}
 
 			result += static_cast <string::value_type>(r);
-
-			if (it != s.end())
-				++it;
-
 			break;
 		}
 		default:
diff --git a/tests/utility/urlTest.cpp b/tests/utility/urlTest.cpp
index f9bb4dae..2630a99e 100644
--- a/tests/utility/urlTest.cpp
+++ b/tests/utility/urlTest.cpp
@@ -42,6 +42,7 @@ VMIME_TEST_SUITE_BEGIN
 		VMIME_TEST(testGenerate)
 		VMIME_TEST(testUtilsEncode)
 		VMIME_TEST(testUtilsDecode)
+		VMIME_TEST(testUtilsDecodeSpecialCases)
 		VMIME_TEST(testUtilsEncodeReservedChars)
 		VMIME_TEST(testUtilsEncodeUnsafeChars)
 	VMIME_TEST_LIST_END
@@ -239,7 +240,7 @@ VMIME_TEST_SUITE_BEGIN
 		{
 			std::ostringstream ossTest;
 			ossTest << "%" << "0123456789ABCDEF"[i / 16]
-				       << "0123456789ABCDEF"[i % 16];
+			               << "0123456789ABCDEF"[i % 16];
 
 			std::ostringstream ossNum;
 			ossNum << i;
@@ -253,6 +254,14 @@ VMIME_TEST_SUITE_BEGIN
 
 	}
 
+	void testUtilsDecodeSpecialCases()
+	{
+		// Bug #1656547: segfault with '%' at the end of the string
+		VASSERT_EQ("1.1", "sadfsda%", vmime::utility::urlUtils::decode("sadfsda%"));
+		VASSERT_EQ("1.2", "sadfsda\x05", vmime::utility::urlUtils::decode("sadfsda%5"));
+		VASSERT_EQ("1.3", "sadfsda\x42", vmime::utility::urlUtils::decode("sadfsda%42"));
+	}
+
 	void testUtilsEncodeReservedChars()
 	{
 		VASSERT_EQ("1",  "%24", vmime::utility::urlUtils::encode("$"));