Skip to content

Commit b89d6fc

Browse files
Jonas Berghefloryd
authored andcommitted
Validate DCP request length
Previously there was no validation of the length field in DCP requests, so if the number of bytes to read in the request was larger than the buffer size the resulting response would contain data from memory outside the buffer. Fixes CVE-2022-26849 Closes #490
1 parent 14f9728 commit b89d6fc

File tree

2 files changed

+70
-0
lines changed

2 files changed

+70
-0
lines changed

src/common/pf_dcp.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -966,6 +966,11 @@ static int pf_dcp_get_set (
966966
src_pos += sizeof (pf_dcp_header_t);
967967
src_dcplen = (src_pos + ntohs (p_src_dcphdr->data_length));
968968

969+
if (src_dcplen > p_buf->len)
970+
{
971+
goto out;
972+
}
973+
969974
p_rsp = pnal_buf_alloc (PF_FRAME_BUFFER_SIZE); /* Get a transmit buffer
970975
for the response */
971976
if (p_rsp == NULL)
@@ -1506,6 +1511,11 @@ static int pf_dcp_identify_req (
15061511
src_pos += sizeof (pf_dcp_header_t);
15071512
src_dcplen = (src_pos + ntohs (p_src_dcphdr->data_length));
15081513

1514+
if (src_dcplen > p_buf->len)
1515+
{
1516+
goto out1;
1517+
}
1518+
15091519
p_rsp = pnal_buf_alloc (PF_FRAME_BUFFER_SIZE); /* Get a transmit buffer
15101520
for the response */
15111521
if (p_rsp == NULL)

test/test_dcp.cpp

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,39 @@ class DcpUnitTest : public PnetUnitTest
4545
{
4646
};
4747

48+
static uint8_t get_wrong_length_req[] = {
49+
// clang-format off
50+
51+
// Destination MAC address. Pos 0
52+
0x12, 0x34, 0x00, 0x78, 0x90, 0xab,
53+
54+
// Source MAC address. Pos 6
55+
0xc8, 0x5b, 0x76, 0xe6, 0x89, 0xdf,
56+
57+
// Ethernet type = Profinet RT. Pos 12
58+
0x88, 0x92,
59+
60+
// DCP set/get. Pos 14
61+
0xfe, 0xfd,
62+
63+
// Get, request. Pos 16
64+
0x03, 0x00,
65+
66+
// XID. Pos 20
67+
0x00, 0x00, 0x00, 0x01,
68+
69+
// Reserved. Pos 22
70+
0x04, 0x01,
71+
72+
// DCP data length. Wrong value. Pos 24
73+
0x04, 0x00,
74+
75+
// Options. Pos 26
76+
0x00, 0x00
77+
78+
// clang-format on
79+
};
80+
4881
static uint8_t get_name_req[] = {
4982
0x12, 0x34, 0x00, 0x78, 0x90, 0xab, 0xc8, 0x5b, 0x76, 0xe6, 0x89, 0xdf,
5083
0x88, 0x92, 0xfe, 0xfd, 0x03, 0x00, 0x00, 0x00, 0x00, 0x05, 0x00, 0x00,
@@ -118,6 +151,33 @@ TEST_F (DcpTest, DcpHelloTest)
118151
EXPECT_EQ (appdata.call_counters.write_calls, 0);
119152
}
120153

154+
TEST_F (DcpTest, DcpGetWrongLengthTest)
155+
{
156+
pnal_buf_t * p_buf;
157+
int ret;
158+
159+
mock_clear();
160+
161+
p_buf = pnal_buf_alloc (PF_FRAME_BUFFER_SIZE);
162+
memcpy (p_buf->payload, get_wrong_length_req, sizeof (get_wrong_length_req));
163+
p_buf->len = sizeof (get_wrong_length_req);
164+
165+
ret = pf_eth_recv (mock_os_data.eth_if_handle, net, p_buf);
166+
167+
EXPECT_EQ (ret, 1); // Incoming frame is handled
168+
EXPECT_EQ (mock_os_data.eth_send_count, 0); // Drop malformed frame
169+
170+
EXPECT_EQ (appdata.call_counters.led_off_calls, 1);
171+
EXPECT_EQ (appdata.call_counters.led_on_calls, 0);
172+
EXPECT_EQ (appdata.call_counters.state_calls, 0);
173+
EXPECT_EQ (appdata.call_counters.connect_calls, 0);
174+
EXPECT_EQ (appdata.call_counters.release_calls, 0);
175+
EXPECT_EQ (appdata.call_counters.dcontrol_calls, 0);
176+
EXPECT_EQ (appdata.call_counters.ccontrol_calls, 0);
177+
EXPECT_EQ (appdata.call_counters.read_calls, 0);
178+
EXPECT_EQ (appdata.call_counters.write_calls, 0);
179+
}
180+
121181
TEST_F (DcpTest, DcpRunTest)
122182
{
123183
pnal_buf_t * p_buf;

0 commit comments

Comments
 (0)