Skip to content

Commit 27de2d4

Browse files
Fix overflow, leak, port validation, and add missing test coverage
- Fix uint16_t overflow in pp2_create_hdr: accumulate TLV lengths in uint32_t, check > UINT16_MAX before truncating - Fix memory leak in test harness: free raw_bytes_in buffer after pp_parse_hdr - Fix MSVC C4244 warning: cast _sprintf return to uint16_t in pp1_create_hdr - Fix parse_port: reject leading + prefix and trailing non-digit characters - Add tests for parse_port strictness (+ prefix, trailing garbage) and pp_info_add_* NULL/length guards
1 parent 7e4b76e commit 27de2d4

2 files changed

Lines changed: 77 additions & 7 deletions

File tree

src/proxy_protocol.c

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ static uint8_t parse_port(const char *value, uint16_t *usport)
217217
{
218218
char *endptr = NULL;
219219
uint64_t port = strtoul(value, &endptr, 10);
220-
if (endptr == value || port > UINT16_MAX)
220+
if (endptr == value || *endptr != '\0' || *value == '+' || port > UINT16_MAX)
221221
{
222222
return 0;
223223
}
@@ -701,7 +701,8 @@ static uint32_t crc32c(const uint8_t* buf, uint32_t len)
701701
static uint8_t *pp2_create_hdr(const pp_info_t *pp_info, uint16_t *pp2_hdr_len, int32_t *error)
702702
{
703703
proxy_hdr_v2_t proxy_hdr_v2 = { PP2_SIG, '\x21', 0, 0 };
704-
uint16_t proxy_addr_len, len, padding_bytes, index;
704+
uint16_t proxy_addr_len, padding_bytes, index;
705+
uint32_t len;
705706
proxy_addr_t proxy_addr;
706707
const tlv_array_t *tlv_array;
707708
uint32_t i;
@@ -782,7 +783,12 @@ static uint8_t *pp2_create_hdr(const pp_info_t *pp_info, uint16_t *pp2_hdr_len,
782783
{
783784
len += sizeof_pp2_tlv_t + sizeof(uint32_t);
784785
}
785-
*pp2_hdr_len = sizeof(proxy_hdr_v2_t) + len;
786+
if (sizeof(proxy_hdr_v2_t) + len > UINT16_MAX)
787+
{
788+
*error = -ERR_PP2_LENGTH;
789+
return NULL;
790+
}
791+
*pp2_hdr_len = (uint16_t)(sizeof(proxy_hdr_v2_t) + len);
786792
if (pp_info->pp2_info.alignment_power > 1)
787793
{
788794
uint16_t alignment = 1 << pp_info->pp2_info.alignment_power;
@@ -794,13 +800,13 @@ static uint8_t *pp2_create_hdr(const pp_info_t *pp_info, uint16_t *pp2_hdr_len,
794800
{
795801
pp2_hdr_len_padded += alignment;
796802
}
797-
padding_bytes = pp2_hdr_len_padded - sizeof(proxy_hdr_v2_t) - len - sizeof_pp2_tlv_t;
803+
padding_bytes = pp2_hdr_len_padded - (uint16_t)sizeof(proxy_hdr_v2_t) - (uint16_t)len - sizeof_pp2_tlv_t;
798804

799805
*pp2_hdr_len = pp2_hdr_len_padded;
800-
len = pp2_hdr_len_padded - sizeof(proxy_hdr_v2_t);
806+
len = pp2_hdr_len_padded - (uint32_t)sizeof(proxy_hdr_v2_t);
801807
}
802808
}
803-
proxy_hdr_v2.len = htons(len);
809+
proxy_hdr_v2.len = htons((uint16_t)len);
804810

805811
/* Create the PROXY protocol header */
806812
pp2_hdr = malloc(*pp2_hdr_len);
@@ -911,7 +917,7 @@ static uint8_t *pp1_create_hdr(const pp_info_t *pp_info, uint16_t *pp1_hdr_len,
911917
}
912918
memcpy(src_addr, pp_info->src_addr, sizeof(src_addr));
913919
memcpy(dst_addr, pp_info->dst_addr, sizeof(dst_addr));
914-
*pp1_hdr_len = _sprintf(block, "PROXY %s %s %s %hu %hu"CRLF, fam, src_addr, dst_addr, pp_info->src_port, pp_info->dst_port);
920+
*pp1_hdr_len = (uint16_t)_sprintf(block, "PROXY %s %s %s %hu %hu"CRLF, fam, src_addr, dst_addr, pp_info->src_port, pp_info->dst_port);
915921
}
916922
else
917923
{

tests/test.c

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -973,6 +973,31 @@ int main(void)
973973
.raw_bytes_in_length = 108,
974974
.rc_expected = -ERR_PP1_CRLF,
975975
},
976+
{
977+
.name = "v1 PROXY protocol header: -ERR_PP1_SRC_PORT (+ prefix)",
978+
.raw_bytes_in = (const uint8_t*) "PROXY TCP4 1.2.3.4 5.6.7.8 +80 443\r\n",
979+
.raw_bytes_in_length = 36,
980+
.rc_expected = -ERR_PP1_SRC_PORT,
981+
.pp_info_out_expected = {
982+
.address_family = ADDR_FAMILY_INET,
983+
.transport_protocol = TRANSPORT_PROTOCOL_STREAM,
984+
.src_addr = "1.2.3.4",
985+
.dst_addr = "5.6.7.8",
986+
},
987+
},
988+
{
989+
.name = "v1 PROXY protocol header: -ERR_PP1_DST_PORT (trailing garbage)",
990+
.raw_bytes_in = (const uint8_t*) "PROXY TCP4 1.2.3.4 5.6.7.8 80 443a\r\n",
991+
.raw_bytes_in_length = 36,
992+
.rc_expected = -ERR_PP1_DST_PORT,
993+
.pp_info_out_expected = {
994+
.address_family = ADDR_FAMILY_INET,
995+
.transport_protocol = TRANSPORT_PROTOCOL_STREAM,
996+
.src_addr = "1.2.3.4",
997+
.dst_addr = "5.6.7.8",
998+
.src_port = 80,
999+
},
1000+
},
9761001
};
9771002

9781003
/* Run tests */
@@ -993,6 +1018,7 @@ int main(void)
9931018
}
9941019
memcpy(buffer, tests[i].raw_bytes_in, tests[i].raw_bytes_in_length);
9951020
pp_parse_hdr_rc = pp_parse_hdr(buffer, tests[i].raw_bytes_in_length, &pp_info_out);
1021+
free(buffer);
9961022
}
9971023
else
9981024
{
@@ -1057,6 +1083,44 @@ int main(void)
10571083
printf("PASSED\n");
10581084
}
10591085

1086+
/* Test pp_info_add_* validation failures */
1087+
printf("Running test: pp_info_add_unique_id length > 128...");
1088+
{
1089+
pp_info_t pp_info = { 0 };
1090+
uint8_t dummy = 0;
1091+
if (pp_info_add_unique_id(&pp_info, 129, &dummy) != 0)
1092+
{
1093+
printf("FAILED\n");
1094+
return EXIT_FAILURE;
1095+
}
1096+
pp_info_clear(&pp_info);
1097+
}
1098+
printf("PASSED\n");
1099+
1100+
printf("Running test: pp_info_add_netns NULL...");
1101+
{
1102+
pp_info_t pp_info = { 0 };
1103+
if (pp_info_add_netns(&pp_info, NULL) != 0)
1104+
{
1105+
printf("FAILED\n");
1106+
return EXIT_FAILURE;
1107+
}
1108+
pp_info_clear(&pp_info);
1109+
}
1110+
printf("PASSED\n");
1111+
1112+
printf("Running test: pp_info_add_aws_vpce_id NULL...");
1113+
{
1114+
pp_info_t pp_info = { 0 };
1115+
if (pp_info_add_aws_vpce_id(&pp_info, NULL) != 0)
1116+
{
1117+
printf("FAILED\n");
1118+
return EXIT_FAILURE;
1119+
}
1120+
pp_info_clear(&pp_info);
1121+
}
1122+
printf("PASSED\n");
1123+
10601124
/* Test pp_strerror() */
10611125
printf("Running test: pp_strerror()...");
10621126
if (strcmp("No error", pp_strerror(ERR_NULL))

0 commit comments

Comments
 (0)