Skip to content

Commit bc6ab2e

Browse files
edsiperedsiper
committed
http_client: reject invalid chunked size lines
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
1 parent 0c3c3bc commit bc6ab2e

2 files changed

Lines changed: 101 additions & 4 deletions

File tree

‎src/flb_http_client.c‎

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,8 @@ static int chunked_data_size(char *buf, size_t length,
377377
{
378378
char *cursor;
379379
char *line_end;
380+
int extension_started;
381+
size_t digit;
380382
size_t digit_count;
381383
size_t line_length;
382384
size_t total_size;
@@ -400,22 +402,28 @@ static int chunked_data_size(char *buf, size_t length,
400402

401403
errno = 0;
402404
digit_count = 0;
405+
extension_started = FLB_FALSE;
403406
value = 0;
404407

405408
while (digit_count < line_length) {
406409
if (*cursor >= '0' && *cursor <= '9') {
407-
value = (value * 16) + (*cursor - '0');
410+
digit = *cursor - '0';
408411
}
409412
else if (*cursor >= 'a' && *cursor <= 'f') {
410-
value = (value * 16) + (*cursor - 'a') + 10;
413+
digit = (*cursor - 'a') + 10;
411414
}
412415
else if (*cursor >= 'A' && *cursor <= 'F') {
413-
value = (value * 16) + (*cursor - 'A') + 10;
416+
digit = (*cursor - 'A') + 10;
414417
}
415418
else {
416419
break;
417420
}
418421

422+
if (value > ((SIZE_MAX - digit) / 16)) {
423+
return FLB_HTTP_ERROR;
424+
}
425+
426+
value = (value * 16) + digit;
419427
digit_count++;
420428
cursor++;
421429
}
@@ -432,10 +440,23 @@ static int chunked_data_size(char *buf, size_t length,
432440
if (digit_count < line_length && *cursor == ';') {
433441
cursor++;
434442
digit_count++;
443+
extension_started = FLB_TRUE;
435444
}
436445

437446
while (digit_count < line_length) {
438-
if (*cursor == '\0') {
447+
if (extension_started == FLB_FALSE) {
448+
if (*cursor != ' ' && *cursor != '\t') {
449+
return FLB_HTTP_ERROR;
450+
}
451+
}
452+
else if (*cursor != ' ' && *cursor != '\t' && *cursor != ';' &&
453+
*cursor != '=' && *cursor != '"' && *cursor != '\\' &&
454+
*cursor != '/' && *cursor != ',' && *cursor != '_' &&
455+
*cursor != '-' && *cursor != '.' && *cursor != ':' &&
456+
*cursor != '(' && *cursor != ')' &&
457+
!(*cursor >= '0' && *cursor <= '9') &&
458+
!(*cursor >= 'a' && *cursor <= 'z') &&
459+
!(*cursor >= 'A' && *cursor <= 'Z')) {
439460
return FLB_HTTP_ERROR;
440461
}
441462

@@ -449,12 +470,20 @@ static int chunked_data_size(char *buf, size_t length,
449470
return FLB_HTTP_OK;
450471
}
451472

473+
if (value > (SIZE_MAX - total_size - 2)) {
474+
return FLB_HTTP_ERROR;
475+
}
476+
452477
total_size += value + 2;
453478

454479
if (length < total_size) {
455480
return FLB_HTTP_MORE;
456481
}
457482

483+
if (value > (length - ((line_end + 2) - buf) - 2)) {
484+
return FLB_HTTP_MORE;
485+
}
486+
458487
if (line_end[2 + value] != '\r' || line_end[3 + value] != '\n') {
459488
return FLB_HTTP_ERROR;
460489
}

‎tests/internal/http_client.c‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -974,6 +974,72 @@ void test_http_response_chunked_multi_stage_trailers()
974974
test_ctx_destroy(ctx);
975975
}
976976

977+
void test_http_response_chunked_invalid_size_suffix()
978+
{
979+
int ret;
980+
struct test_ctx *ctx;
981+
struct flb_http_client *c;
982+
983+
ctx = test_ctx_create();
984+
if (!TEST_CHECK(ctx != NULL)) {
985+
exit(EXIT_FAILURE);
986+
}
987+
988+
c = flb_http_client(ctx->u_conn, FLB_HTTP_GET, "/", NULL, 0,
989+
"127.0.0.1", 80, NULL, FLB_HTTP_11);
990+
if (!TEST_CHECK(c != NULL)) {
991+
TEST_MSG("flb_http_client failed");
992+
test_ctx_destroy(ctx);
993+
exit(EXIT_FAILURE);
994+
}
995+
996+
append_response_fragment(c,
997+
"HTTP/1.1 200 OK\r\n"
998+
"Transfer-Encoding: chunked\r\n"
999+
"\r\n"
1000+
"1g\r\n"
1001+
"A\r\n"
1002+
"0\r\n\r\n");
1003+
1004+
ret = flb_http_client_process_response_buffer(c);
1005+
TEST_CHECK(ret == FLB_HTTP_ERROR);
1006+
1007+
flb_http_client_destroy(c);
1008+
test_ctx_destroy(ctx);
1009+
}
1010+
1011+
void test_http_response_chunked_oversized_length()
1012+
{
1013+
int ret;
1014+
struct test_ctx *ctx;
1015+
struct flb_http_client *c;
1016+
1017+
ctx = test_ctx_create();
1018+
if (!TEST_CHECK(ctx != NULL)) {
1019+
exit(EXIT_FAILURE);
1020+
}
1021+
1022+
c = flb_http_client(ctx->u_conn, FLB_HTTP_GET, "/", NULL, 0,
1023+
"127.0.0.1", 80, NULL, FLB_HTTP_11);
1024+
if (!TEST_CHECK(c != NULL)) {
1025+
TEST_MSG("flb_http_client failed");
1026+
test_ctx_destroy(ctx);
1027+
exit(EXIT_FAILURE);
1028+
}
1029+
1030+
append_response_fragment(c,
1031+
"HTTP/1.1 200 OK\r\n"
1032+
"Transfer-Encoding: chunked\r\n"
1033+
"\r\n"
1034+
"ffffffffffffffffffffffffffffffff\r\n");
1035+
1036+
ret = flb_http_client_process_response_buffer(c);
1037+
TEST_CHECK(ret == FLB_HTTP_ERROR);
1038+
1039+
flb_http_client_destroy(c);
1040+
test_ctx_destroy(ctx);
1041+
}
1042+
9771043
void test_http_timeout_setters_preserve_upstream_io_timeout()
9781044
{
9791045
struct test_ctx *ctx;
@@ -1040,6 +1106,8 @@ TEST_LIST = {
10401106
{ "response_chunked_empty_terminal_split", test_http_response_chunked_empty_terminal_split},
10411107
{ "response_chunked_uppercase_hex_whitespace", test_http_response_chunked_uppercase_hex_whitespace},
10421108
{ "response_chunked_multi_stage_trailers", test_http_response_chunked_multi_stage_trailers},
1109+
{ "response_chunked_invalid_size_suffix", test_http_response_chunked_invalid_size_suffix},
1110+
{ "response_chunked_oversized_length", test_http_response_chunked_oversized_length},
10431111
{ "timeout_setters_preserve_upstream_io_timeout", test_http_timeout_setters_preserve_upstream_io_timeout},
10441112
{ 0 }
10451113
};

0 commit comments

Comments
 (0)