Skip to content

Commit ec973dc

Browse files
authored
Fix heap buffer overflow (etr#215)
* Fix potential issue with heap buffer overflow. Thanks to sgbhat2 for fuzztesting the library and finding out. * Added unit test to cover new codepath in unescaping
1 parent 7cb4eb8 commit ec973dc

File tree

2 files changed

+24
-6
lines changed

2 files changed

+24
-6
lines changed

src/http_utils.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333

3434
#include <stdio.h>
3535
#include <stdlib.h>
36+
#include <ctype.h>
3637
#include <fstream>
3738
#include <iomanip>
3839
#include <iterator>
@@ -304,12 +305,13 @@ size_t http_unescape(std::string& val)
304305
{
305306
if (val.empty()) return 0;
306307

307-
int rpos = 0;
308-
int wpos = 0;
308+
unsigned int rpos = 0;
309+
unsigned int wpos = 0;
309310

310311
unsigned int num;
312+
unsigned int size = val.size();
311313

312-
while ('\0' != val[rpos])
314+
while (rpos < size && val[rpos] != '\0')
313315
{
314316
switch (val[rpos])
315317
{
@@ -319,8 +321,8 @@ size_t http_unescape(std::string& val)
319321
rpos++;
320322
break;
321323
case '%':
322-
if ( (1 == sscanf (val.substr(rpos + 1).c_str(), "%2x", &num)) ||
323-
(1 == sscanf (val.substr(rpos + 1).c_str(), "%2X", &num))
324+
if (size > rpos + 2 && ((1 == sscanf (val.substr(rpos + 1, 2).c_str(), "%2x", &num)) ||
325+
(1 == sscanf (val.substr(rpos + 1, 2).c_str(), "%2X", &num)))
324326
)
325327
{
326328
val[wpos] = (unsigned char) num;

test/unit/http_utils_test.cpp

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ LT_BEGIN_AUTO_TEST(http_utils_suite, unescape)
6767
LT_END_AUTO_TEST(unescape)
6868

6969
LT_BEGIN_AUTO_TEST(http_utils_suite, unescape_plus)
70-
char* with_plus = (char*) malloc(6 * sizeof(char));
70+
char* with_plus = (char*) malloc(4 * sizeof(char));
7171
sprintf(with_plus, "%s", "A+B");
7272
std::string string_with_plus = with_plus;
7373
int expected_size = http::http_unescape(string_with_plus);
@@ -82,6 +82,22 @@ LT_BEGIN_AUTO_TEST(http_utils_suite, unescape_plus)
8282
free(expected);
8383
LT_END_AUTO_TEST(unescape_plus)
8484

85+
LT_BEGIN_AUTO_TEST(http_utils_suite, unescape_partial_marker)
86+
char* with_marker = (char*) malloc(6 * sizeof(char));
87+
sprintf(with_marker, "%s", "A+B%0");
88+
std::string string_with_marker = with_marker;
89+
int expected_size = http::http_unescape(string_with_marker);
90+
91+
char* expected = (char*) malloc(6 * sizeof(char));
92+
sprintf(expected, "%s", "A B%0");
93+
94+
LT_CHECK_EQ(string_with_marker, string(expected));
95+
LT_CHECK_EQ(expected_size, 5);
96+
97+
free(with_marker);
98+
free(expected);
99+
LT_END_AUTO_TEST(unescape_partial_marker)
100+
85101
LT_BEGIN_AUTO_TEST(http_utils_suite, tokenize_url)
86102
string value = "test/this/url/here";
87103
string expected_arr[] = { "test", "this", "url", "here" };

0 commit comments

Comments
 (0)