Skip to content

Commit d685890

Browse files
etretr
committed
Eliminate unsafe access to char*
1 parent c9b0c8f commit d685890

File tree

8 files changed

+52
-43
lines changed

8 files changed

+52
-43
lines changed

‎src/http_utils.cpp‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -281,42 +281,44 @@ unsigned short get_port(const struct sockaddr* sa)
281281
}
282282
}
283283

284-
size_t http_unescape(char *val)
284+
size_t http_unescape(std::string& val)
285285
{
286-
char *rpos = val;
287-
char *wpos = val;
286+
if (val.empty()) return 0;
287+
288+
int rpos = 0;
289+
int wpos = 0;
290+
288291
unsigned int num;
289292

290-
while ('\0' != *rpos)
293+
while ('\0' != val[rpos])
291294
{
292-
switch (*rpos)
295+
switch (val[rpos])
293296
{
294297
case '+':
295-
*wpos = ' ';
298+
val[wpos] = ' ';
296299
wpos++;
297300
rpos++;
298301
break;
299302
case '%':
300-
if ( (1 == sscanf (&rpos[1],
301-
"%2x", &num)) ||
302-
(1 == sscanf (&rpos[1],
303-
"%2X", &num))
303+
if ( (1 == sscanf (val.substr(rpos + 1).c_str(), "%2x", &num)) ||
304+
(1 == sscanf (val.substr(rpos + 1).c_str(), "%2X", &num))
304305
)
305306
{
306-
*wpos = (unsigned char) num;
307+
val[wpos] = (unsigned char) num;
307308
wpos++;
308309
rpos += 3;
309310
break;
310311
}
311312
/* intentional fall through! */
312313
default:
313-
*wpos = *rpos;
314+
val[wpos] = val[rpos];
314315
wpos++;
315316
rpos++;
316317
}
317318
}
318-
*wpos = '\0'; /* add 0-terminator */
319-
return wpos - val; /* = strlen(val) */
319+
val[wpos] = '\0'; /* add 0-terminator */
320+
val.resize(wpos);
321+
return wpos; /* = strlen(val) */
320322
}
321323

322324
ip_representation::ip_representation(const struct sockaddr* ip)

‎src/httpserver/create_webserver.hpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ class http_request;
3939

4040
typedef const http_response(*render_ptr)(const http_request&);
4141
typedef bool(*validator_ptr)(const std::string&);
42-
typedef void(*unescaper_ptr)(char*);
42+
typedef void(*unescaper_ptr)(std::string&);
4343
typedef void(*log_access_ptr)(const std::string&);
4444
typedef void(*log_error_ptr)(const std::string&);
4545

‎src/httpserver/http_utils.hpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,7 @@ void dump_arg_map(std::ostream &os, const std::string &prefix,
352352
* @return length of the resulting val (strlen(val) maybe
353353
* shorter afterwards due to elimination of escape sequences)
354354
*/
355-
size_t http_unescape (char *val);
355+
size_t http_unescape (std::string& val);
356356

357357
const std::string load_file (const std::string& filename);
358358

‎src/httpserver/webserver.hpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ class webserver
288288
friend size_t unescaper_func(void * cls,
289289
struct MHD_Connection *c, char *s
290290
);
291-
friend size_t internal_unescaper(void * cls, char *s);
291+
friend size_t internal_unescaper(void * cls, std::string& s);
292292
friend class http_response;
293293
};
294294

‎src/string_utilities.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,12 +107,12 @@ const std::string regex_replace(const std::string& str,
107107
);
108108

109109
memcpy(&ns[substmatch[0].rm_so+replace_str.size()],
110-
&str[substmatch[0].rm_eo], strlen(&str[substmatch[0].rm_eo])
110+
&str[substmatch[0].rm_eo], str.substr(substmatch[0].rm_eo).size()
111111
);
112112

113113
ns[substmatch[0].rm_so +
114114
replace_str.size() +
115-
strlen(&str[substmatch[0].rm_eo])
115+
str.substr(substmatch[0].rm_eo).size()
116116
] = 0;
117117

118118
result = std::string((char*)ns);

‎src/webserver.cpp‎

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ void error_log(void*, const char*, va_list);
9696
void* uri_log(void*, const char*);
9797
void access_log(webserver*, string);
9898
size_t unescaper_func(void*, struct MHD_Connection*, char*);
99-
size_t internal_unescaper(void*, char*);
99+
size_t internal_unescaper(void*, std::string&);
100100

101101
struct compare_value
102102
{
@@ -494,22 +494,22 @@ int webserver::build_request_args (
494494
)
495495
{
496496
details::modded_request* mr = static_cast<details::modded_request*>(cls);
497-
char* value = (char*) ((arg_value == NULL) ? "" : arg_value);
497+
std::string value = ((arg_value == NULL) ? "" : arg_value);
498498
{
499-
char buf[strlen(key) + strlen(value) + 3];
499+
char buf[strlen(key) + value.size() + 3];
500500
if(mr->dhr->querystring == "")
501501
{
502-
snprintf(buf, sizeof buf, "?%s=%s", key, value);
502+
snprintf(buf, sizeof buf, "?%s=%s", key, value.c_str());
503503
mr->dhr->querystring = buf;
504504
}
505505
else
506506
{
507-
snprintf(buf, sizeof buf, "&%s=%s", key, value);
507+
snprintf(buf, sizeof buf, "&%s=%s", key, value.c_str());
508508
mr->dhr->querystring += string(buf);
509509
}
510510
}
511-
size_t size = internal_unescaper((void*) mr->ws, value);
512-
mr->dhr->set_arg(key, string(value, size));
511+
internal_unescaper((void*) mr->ws, value);
512+
mr->dhr->set_arg(key, value);
513513
return MHD_YES;
514514
}
515515

@@ -560,15 +560,15 @@ size_t unescaper_func(void * cls, struct MHD_Connection *c, char *s)
560560
return strlen(s);
561561
}
562562

563-
size_t internal_unescaper(void* cls, char* s)
563+
size_t internal_unescaper(void* cls, std::string& s)
564564
{
565565
if(s[0] == 0) return 0;
566566

567567
webserver* dws = static_cast<webserver*>(cls);
568568
if(dws->unescaper != 0x0)
569569
{
570570
dws->unescaper(s);
571-
return strlen(s);
571+
return s.size();
572572
}
573573

574574
return http_unescape(s);
@@ -658,15 +658,15 @@ int webserver::bodyfull_requests_answer_first_step(
658658
(
659659
0x0 != encoding &&
660660
((0 == strncasecmp (
661-
MHD_HTTP_POST_ENCODING_FORM_URLENCODED,
661+
http_utils::http_post_encoding_form_urlencoded.c_str(),
662662
encoding,
663-
strlen (MHD_HTTP_POST_ENCODING_FORM_URLENCODED)
663+
http_utils::http_post_encoding_form_urlencoded.size()
664664
)
665665
)
666666
|| (0 == strncasecmp (
667-
MHD_HTTP_POST_ENCODING_MULTIPART_FORMDATA,
667+
http_utils::http_post_encoding_multipart_formdata.c_str(),
668668
encoding,
669-
strlen (MHD_HTTP_POST_ENCODING_MULTIPART_FORMDATA)
669+
http_utils::http_post_encoding_multipart_formdata.size()
670670
)))
671671
)
672672
)
@@ -966,8 +966,10 @@ int webserver::answer_to_connection(void* cls, MHD_Connection* connection,
966966
);
967967
}
968968

969-
internal_unescaper((void*) static_cast<webserver*>(cls), (char*) url);
970-
mr->standardized_url = new string(http_utils::standardize_url(url));
969+
std::string t_url = url;
970+
971+
internal_unescaper((void*) static_cast<webserver*>(cls), t_url);
972+
mr->standardized_url = new string(http_utils::standardize_url(t_url));
971973

972974
bool body = false;
973975

‎test/integ/deferred.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ ssize_t test_callback (char* buf, size_t max)
5858
memset(buf, 0, max);
5959
strcat(buf, "test");
6060
counter++;
61-
return strlen(buf);
61+
return std::string(buf).size();
6262
}
6363
}
6464

‎test/unit/http_utils_test.cpp‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,32 +47,37 @@ LT_BEGIN_SUITE(http_utils_suite)
4747
LT_END_SUITE(http_utils_suite)
4848

4949
LT_BEGIN_AUTO_TEST(http_utils_suite, unescape)
50-
char* string_with_plus = (char*) malloc(6 * sizeof(char));
51-
sprintf(string_with_plus, "%s", "A%20B");
50+
char* with_plus = (char*) malloc(6 * sizeof(char));
51+
sprintf(with_plus, "%s", "A%20B");
52+
std::string string_with_plus = with_plus;
5253
int expected_size = http::http_unescape(string_with_plus);
5354

5455
char* expected = (char*) malloc(4 * sizeof(char));
5556
sprintf(expected, "%s", "A B");
5657

57-
LT_CHECK_EQ(string(string_with_plus), string(expected));
58+
std::cout << "|||||" << string_with_plus << "||||" << std::endl;
59+
std::cout << expected << std::endl;
60+
61+
LT_CHECK_EQ(string_with_plus, string(expected));
5862
LT_CHECK_EQ(expected_size, 3);
5963

60-
free(string_with_plus);
64+
free(with_plus);
6165
free(expected);
6266
LT_END_AUTO_TEST(unescape)
6367

6468
LT_BEGIN_AUTO_TEST(http_utils_suite, unescape_plus)
65-
char* string_with_plus = (char*) malloc(6 * sizeof(char));
66-
sprintf(string_with_plus, "%s", "A+B");
69+
char* with_plus = (char*) malloc(6 * sizeof(char));
70+
sprintf(with_plus, "%s", "A+B");
71+
std::string string_with_plus = with_plus;
6772
int expected_size = http::http_unescape(string_with_plus);
6873

6974
char* expected = (char*) malloc(4 * sizeof(char));
7075
sprintf(expected, "%s", "A B");
7176

72-
LT_CHECK_EQ(string(string_with_plus), string(expected));
77+
LT_CHECK_EQ(string_with_plus, string(expected));
7378
LT_CHECK_EQ(expected_size, 3);
7479

75-
free(string_with_plus);
80+
free(with_plus);
7681
free(expected);
7782
LT_END_AUTO_TEST(unescape_plus)
7883

0 commit comments

Comments
 (0)