Skip to content

Commit 056d1d6

Browse files
committed
Fix buffer overflows, double-free, and test bugs
Add bounded variants of XOR decode and message extraction functions that check computed message length against actual buffer size before reading, preventing stack-buffer-overflows when trying wrong XOR keys. Fix double-free in phev_pipe_outputChainInputTransformer where both the transformer and the framework freed the same message on decode failure. Now frees only the locally-allocated phevMessage and lets the framework handle the original message lifecycle. Fix test_phev_pipe_updateRegisterWithCallback_encoded to set commandXOR alongside currentXOR, matching the corrected XOR key management from commit 22ac108. Fix test_phev_service_end_to_end_operations to verify only the last published message since the test output handler overwrites on each call. Fix test_core_phev_core_extractIncomingMessageValidFirstByteCommand to assert raw XOR-encoded bytes and correct XOR key value. Migrate all test callers of extractIncomingMessageAndXOR and extractAndDecodeIncomingMessageAndXOR to bounded variants to eliminate ASAN-detected stack overflows with small test buffers.
1 parent 38a507e commit 056d1d6

6 files changed

Lines changed: 155 additions & 22 deletions

File tree

include/phev_core.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,10 +149,14 @@ bool phev_core_validateChecksum(const uint8_t *data);
149149

150150
message_t * phev_core_extractIncomingMessageAndXOR(const uint8_t * data);
151151

152+
message_t * phev_core_extractIncomingMessageAndXORBounded(const uint8_t * data, const size_t bufLen);
153+
152154
message_t * phev_core_extractOutgoingMessageAndXOR(const uint8_t * data);
153155

154156
message_t * phev_core_extractAndDecodeIncomingMessageAndXOR(const uint8_t *data);
155157

158+
message_t * phev_core_extractAndDecodeIncomingMessageAndXORBounded(const uint8_t *data, const size_t bufLen);
159+
156160
message_t * phev_core_extractAndDecodeOutgoingMessageAndXOR(const uint8_t *data);
157161

158162
message_t * phev_core_createMsgXOR(const uint8_t * data, const size_t length, const uint8_t xor);

src/phev_core.c

Lines changed: 130 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,38 @@ uint8_t *phev_core_xorDataWithValue(const uint8_t *data, const uint8_t xor)
2828

2929
return decoded;
3030
}
31+
static uint8_t *phev_core_xorDataWithValueBounded(const uint8_t *data, const uint8_t xor, const size_t bufLen)
32+
{
33+
LOG_V(APP_TAG, "START - xorDataWithValueBounded");
34+
35+
if (bufLen < 2)
36+
{
37+
LOG_E(APP_TAG, "Buffer too small for XOR decode");
38+
return NULL;
39+
}
40+
41+
size_t length = (data[1] ^ xor) + 2;
42+
43+
if (length > bufLen)
44+
{
45+
LOG_D(APP_TAG, "Computed length %zu exceeds buffer size %zu with XOR %02X", length, bufLen, xor);
46+
return NULL;
47+
}
48+
49+
uint8_t *decoded = malloc(length);
50+
51+
LOG_D(APP_TAG, "Decoding data with length %d with XOR %02X", length, xor);
52+
53+
for (int i = 0; i < length; i++)
54+
{
55+
decoded[i] = data[i] ^ xor;
56+
}
57+
58+
LOG_BUFFER_HEXDUMP(APP_TAG, decoded, length, LOG_DEBUG);
59+
LOG_V(APP_TAG, "END - xorDataWithValueBounded");
60+
61+
return decoded;
62+
}
3163
bool phev_core_checkIncomingCommand(const uint8_t command)
3264
{
3365
switch (command)
@@ -103,6 +135,21 @@ bool phev_core_validateChecksumXOR(const uint8_t *data, const uint8_t xor)
103135

104136
return valid;
105137
}
138+
static bool phev_core_validateChecksumXORBounded(const uint8_t *data, const uint8_t xor, const size_t bufLen)
139+
{
140+
uint8_t *decodedData = phev_core_xorDataWithValueBounded(data, xor, bufLen);
141+
142+
if (decodedData == NULL)
143+
{
144+
return false;
145+
}
146+
147+
bool valid = phev_core_validateChecksum(decodedData);
148+
149+
free(decodedData);
150+
151+
return valid;
152+
}
106153
message_t *phev_core_unencodedIncomingMessage(const uint8_t *data)
107154
{
108155
uint8_t command = data[0];
@@ -212,6 +259,37 @@ message_t *phev_core_encodedIncomingMessage(const uint8_t *data)
212259

213260
return NULL;
214261
}
262+
static message_t *phev_core_encodedIncomingMessageBounded(const uint8_t *data, const size_t bufLen)
263+
{
264+
if (bufLen < 3)
265+
{
266+
LOG_E(APP_TAG, "Buffer too small for encoded message decode");
267+
return NULL;
268+
}
269+
270+
uint8_t xor = data[2];
271+
uint8_t command = data[0] ^ xor;
272+
uint8_t length = (data[1] ^ xor) + 2;
273+
274+
if (length <= bufLen && phev_core_checkIncomingCommand(command) && phev_core_validateChecksumXORBounded(data, xor, bufLen))
275+
{
276+
message_t * message = phev_core_createMsgXOR(data,length,xor);
277+
return message;
278+
}
279+
280+
xor ^= 1;
281+
command = data[0] ^ xor;
282+
length = (data[1] ^ xor) + 2;
283+
284+
if (length <= bufLen && phev_core_checkIncomingCommand(command) && phev_core_validateChecksumXORBounded(data, xor, bufLen))
285+
{
286+
return phev_core_createMsgXOR(data, length, xor);
287+
}
288+
289+
LOG_E(APP_TAG,"Unknown encoded command %02X or %02X", command, command ^ 1);
290+
291+
return NULL;
292+
}
215293
message_t *phev_core_encodedOutgoingMessage(const uint8_t *data)
216294
{
217295
uint8_t xor = data[2];
@@ -256,6 +334,31 @@ message_t * phev_core_extractIncomingMessageAndXOR(const uint8_t *data)
256334

257335
return message;
258336
}
337+
message_t * phev_core_extractIncomingMessageAndXORBounded(const uint8_t *data, const size_t bufLen)
338+
{
339+
LOG_V(APP_TAG, "START - extractIncomingMessageAndXORBounded");
340+
341+
if (bufLen < 3)
342+
{
343+
LOG_E(APP_TAG, "Buffer too small for message extraction");
344+
return NULL;
345+
}
346+
347+
message_t *message = NULL;
348+
349+
if (phev_core_checkIncomingCommand(data[0]) && phev_core_validateChecksumXORBounded(data, 0, bufLen))
350+
{
351+
message = phev_core_unencodedIncomingMessage(data);
352+
}
353+
else
354+
{
355+
message = phev_core_encodedIncomingMessageBounded(data, bufLen);
356+
}
357+
358+
LOG_V(APP_TAG, "END - extractIncomingMessageAndXORBounded");
359+
360+
return message;
361+
}
259362
message_t * phev_core_extractOutgoingMessageAndXOR(const uint8_t *data)
260363
{
261364
LOG_V(APP_TAG, "START - extractOutgoingMessageAndXOR");
@@ -322,6 +425,32 @@ message_t * phev_core_extractAndDecodeIncomingMessageAndXOR(const uint8_t *data)
322425

323426
return decoded;
324427
}
428+
message_t * phev_core_extractAndDecodeIncomingMessageAndXORBounded(const uint8_t *data, const size_t bufLen)
429+
{
430+
LOG_V(APP_TAG, "START - extractAndDecodeIncomingMessageAndXORBounded");
431+
432+
message_t * message = phev_core_extractIncomingMessageAndXORBounded(data, bufLen);
433+
434+
if(message == NULL)
435+
{
436+
LOG_W(APP_TAG,"Cannot extract incoming message");
437+
return NULL;
438+
}
439+
440+
uint8_t xor = phev_core_getMessageXOR(message);
441+
442+
uint8_t * decodedData = phev_core_xorDataWithValue(message->data, xor);
443+
444+
message_t * decoded = phev_core_createMsgXOR(decodedData,message->length,xor);
445+
446+
free(decodedData);
447+
448+
msg_utils_destroyMsg(message);
449+
450+
LOG_V(APP_TAG, "END - extractAndDecodeIncomingMessageAndXORBounded");
451+
452+
return decoded;
453+
}
325454
message_t * phev_core_extractAndDecodeOutgoingMessageAndXOR(const uint8_t *data)
326455
{
327456
LOG_V(APP_TAG, "START - extractAndDecodeOutgoingMessageAndXOR");
@@ -558,7 +687,7 @@ int phev_core_decodeMessage(const uint8_t *data, const size_t len, phevMessage_t
558687
return 0;
559688
}
560689

561-
message_t * message = phev_core_extractAndDecodeIncomingMessageAndXOR(data);
690+
message_t * message = phev_core_extractAndDecodeIncomingMessageAndXORBounded(data, len);
562691

563692
if (message)
564693
{

src/phev_pipe.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ message_t *phev_pipe_outputChainInputTransformer(void *ctx, message_t *message)
238238
{
239239
LOG_E(APP_TAG, "Invalid message received");
240240

241-
msg_utils_destroyMsg(message);
241+
free(phevMessage);
242242
return NULL;
243243
}
244244
if(message->ctx != NULL)
@@ -820,7 +820,7 @@ messageBundle_t *phev_pipe_outputSplitter(void *ctx, message_t *message)
820820
}
821821
LOG_BUFFER_HEXDUMP(APP_TAG, message->data, message->length, LOG_DEBUG);
822822

823-
message_t * out = phev_core_extractIncomingMessageAndXOR(message->data);
823+
message_t * out = phev_core_extractIncomingMessageAndXORBounded(message->data, message->length);
824824

825825
if (out == NULL)
826826
{
@@ -843,7 +843,7 @@ messageBundle_t *phev_pipe_outputSplitter(void *ctx, message_t *message)
843843

844844
while (message->length > total)
845845
{
846-
out = phev_core_extractIncomingMessageAndXOR(message->data + total);
846+
out = phev_core_extractIncomingMessageAndXORBounded(message->data + total, message->length - total);
847847
if (out == NULL) {
848848
break;
849849
}

test/test_phev_core.c

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ void test_phev_core_extractAndDecodeIncomingMessageAndXOR(void)
3232
uint8_t input[] = { 0x4f,0x26,0x20,0x23,0x21,0x31,0x43,0xcd };
3333
uint8_t expected[] = { 0x6f,0x06,0x00,0x03,0x01,0x11,0x63,0xed};
3434

35-
message_t * message = phev_core_extractAndDecodeIncomingMessageAndXOR(input);
35+
message_t * message = phev_core_extractAndDecodeIncomingMessageAndXORBounded(input, sizeof(input));
3636

3737
TEST_ASSERT_NOT_NULL(message);
3838
TEST_ASSERT_EQUAL_MEMORY(expected,message->data,sizeof(expected));
@@ -792,7 +792,7 @@ void test_phev_core_decodeMessage_command_response(void)
792792
void test_core_phev_core_extractIncomingMessageAndXOR_valid_ping_in_clear(void)
793793
{
794794
uint8_t input[] = { 0x3f,0x04,0x01,0x00,0x00,0x44 };
795-
message_t * message = phev_core_extractIncomingMessageAndXOR(input);
795+
message_t * message = phev_core_extractIncomingMessageAndXORBounded(input, sizeof(input));
796796

797797
TEST_ASSERT_NOT_NULL(message);
798798
TEST_ASSERT_EQUAL_HEX8_ARRAY(input,message->data,sizeof(input));
@@ -801,7 +801,7 @@ void test_core_phev_core_extractIncomingMessageAndXOR_valid_ping_in_clear(void)
801801
void test_core_phev_core_extractIncomingMessageAndXOR_valid_ping_encoded(void)
802802
{
803803
uint8_t input[] = { 0xa1,0x9a,0x9f,0x96,0x9e,0xd2 };
804-
message_t * message = phev_core_extractIncomingMessageAndXOR(input);
804+
message_t * message = phev_core_extractIncomingMessageAndXORBounded(input, sizeof(input));
805805

806806
TEST_ASSERT_NOT_NULL(message);
807807
TEST_ASSERT_EQUAL_HEX8_ARRAY(input,message->data,sizeof(input));
@@ -811,7 +811,7 @@ void test_core_phev_core_extractIncomingMessageAndXOR_valid_ping_encoded(void)
811811
void test_core_phev_core_extractIncomingMessageAndXOR_valid_command_response_in_clear(void)
812812
{
813813
uint8_t input[] = { 0x6F,0x04,0x01,0x07,0x00,0x7B };
814-
message_t * message = phev_core_extractIncomingMessageAndXOR(input);
814+
message_t * message = phev_core_extractIncomingMessageAndXORBounded(input, sizeof(input));
815815

816816
TEST_ASSERT_NOT_NULL(message);
817817
TEST_ASSERT_EQUAL_HEX8_ARRAY(input,message->data,sizeof(input));
@@ -820,7 +820,7 @@ void test_core_phev_core_extractIncomingMessageAndXOR_valid_command_response_in_
820820
void test_core_phev_core_extractIncomingMessageAndXOR_valid_command_response_encoded(void)
821821
{
822822
uint8_t input[] = { 0x5F,0x34,0x31,0x35,0x30,0x49 }; // 6F 04 01 05 00 79
823-
message_t * message = phev_core_extractIncomingMessageAndXOR(input);
823+
message_t * message = phev_core_extractIncomingMessageAndXORBounded(input, sizeof(input));
824824

825825
TEST_ASSERT_NOT_NULL(message);
826826
TEST_ASSERT_EQUAL_HEX8_ARRAY(input,message->data,sizeof(input));
@@ -830,7 +830,7 @@ void test_core_phev_core_extractIncomingMessageAndXOR_valid_command_response_enc
830830
void test_core_phev_core_extractIncomingMessageAndXOR_valid_command_request_in_clear(void)
831831
{
832832
uint8_t input[] = { 0x6F,0x04,0x00,0x1B,0x01,0x8F };
833-
message_t * message = phev_core_extractIncomingMessageAndXOR(input);
833+
message_t * message = phev_core_extractIncomingMessageAndXORBounded(input, sizeof(input));
834834

835835
TEST_ASSERT_NOT_NULL(message);
836836
TEST_ASSERT_EQUAL_HEX8_ARRAY(input,message->data,sizeof(input));
@@ -839,7 +839,7 @@ void test_core_phev_core_extractIncomingMessageAndXOR_valid_command_request_in_c
839839
void test_core_phev_core_extractIncomingMessageAndXOR_valid_command_request_encoded(void)
840840
{
841841
uint8_t input[] = { 0xF1,0x9A,0x9E,0x85,0x9F,0x11 }; // 6F 04 00 1B 01 8F
842-
message_t * message = phev_core_extractIncomingMessageAndXOR(input);
842+
message_t * message = phev_core_extractIncomingMessageAndXORBounded(input, sizeof(input));
843843

844844
TEST_ASSERT_NOT_NULL(message);
845845
TEST_ASSERT_EQUAL_HEX8_ARRAY(input,message->data,sizeof(input));
@@ -849,7 +849,7 @@ void test_core_phev_core_extractIncomingMessageAndXOR_valid_command_request_enco
849849
void test_core_phev_core_extractIncomingMessageAndXOR_valid_command_start_in_clear(void)
850850
{
851851
uint8_t input[] = { 0x4E,0x0C,0x00,0x01,0x04,0x69,0x1D,0x04,0x61,0x94,0xF2,0x3F,0x02,0x11 };
852-
message_t * message = phev_core_extractIncomingMessageAndXOR(input);
852+
message_t * message = phev_core_extractIncomingMessageAndXORBounded(input, sizeof(input));
853853

854854
TEST_ASSERT_NOT_NULL(message);
855855
TEST_ASSERT_EQUAL_HEX8_ARRAY(input,message->data,sizeof(input));
@@ -859,14 +859,14 @@ void test_core_phev_core_extractIncomingMessageAndXOR_valid_command_start_in_cle
859859
void test_core_phev_core_extractIncomingMessageAndXOR_invalid_command(void)
860860
{
861861
uint8_t input[] = { 0x4F,0x0C,0x00,0x01,0x04,0x69,0x1D,0x04,0x61,0x94,0xF2,0x3F,0x02,0x11 };
862-
message_t * message = phev_core_extractIncomingMessageAndXOR(input);
862+
message_t * message = phev_core_extractIncomingMessageAndXORBounded(input, sizeof(input));
863863

864864
TEST_ASSERT_NULL(message);
865865
}
866866
void test_core_phev_core_extractIncomingMessageAndXOR_BB_command(void)
867867
{
868868
uint8_t input[] = { 0xB1,0x0E,0x0B,0x91,0x00,0x6F };
869-
message_t * message = phev_core_extractIncomingMessageAndXOR(input);
869+
message_t * message = phev_core_extractIncomingMessageAndXORBounded(input, sizeof(input));
870870

871871
TEST_ASSERT_NOT_NULL(message);
872872
TEST_ASSERT_EQUAL_HEX8_ARRAY(input,message->data,sizeof(input));
@@ -875,7 +875,7 @@ void test_core_phev_core_extractIncomingMessageAndXOR_BB_command(void)
875875
void test_core_phev_core_extractIncomingMessageAndXOR_CC_command(void)
876876
{
877877
uint8_t input[] = {0xDE,0x16,0x13,0xC4,0x3B,0xC2 };
878-
message_t * message = phev_core_extractIncomingMessageAndXOR(input);
878+
message_t * message = phev_core_extractIncomingMessageAndXORBounded(input, sizeof(input));
879879

880880
TEST_ASSERT_NOT_NULL(message);
881881
TEST_ASSERT_EQUAL_HEX8_ARRAY(input,message->data,sizeof(input));
@@ -884,7 +884,7 @@ void test_core_phev_core_extractIncomingMessageAndXOR_CC_command(void)
884884
void test_core_phev_core_extractIncomingMessageAndXOR_2F_command(void)
885885
{
886886
uint8_t input[] = {0x3A,0x16,0x15,0x14,0x26 };
887-
message_t * message = phev_core_extractIncomingMessageAndXOR(input);
887+
message_t * message = phev_core_extractIncomingMessageAndXORBounded(input, sizeof(input));
888888

889889
TEST_ASSERT_NOT_NULL(message);
890890
TEST_ASSERT_EQUAL_HEX8_ARRAY(input,message->data,sizeof(input));
@@ -893,12 +893,11 @@ void test_core_phev_core_extractIncomingMessageAndXOR_2F_command(void)
893893
void test_core_phev_core_extractIncomingMessageValidFirstByteCommand(void)
894894
{
895895
uint8_t input[]= {0x6f,0xa7,0xa2,0x8b,0x62,0x19};
896-
uint8_t expected[] = {0xcc,0x04,0x28,0x8b,0xba};
897-
message_t * message = phev_core_extractIncomingMessageAndXOR(input);
896+
message_t * message = phev_core_extractIncomingMessageAndXORBounded(input, sizeof(input));
898897

899898
TEST_ASSERT_NOT_NULL(message);
900-
TEST_ASSERT_EQUAL_HEX8_ARRAY(expected,message->data,sizeof(expected));
901-
899+
TEST_ASSERT_EQUAL_HEX8_ARRAY(input,message->data,sizeof(input));
900+
TEST_ASSERT_EQUAL(0xa3,phev_core_getMessageXOR(message));
902901
}
903902
/*
904903
void test_phev_core_decode_encode(void)

test/test_phev_pipe.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1229,6 +1229,7 @@ void test_phev_pipe_updateRegisterWithCallback_encoded(void)
12291229
};
12301230
phev_pipe_ctx_t * ctx = phev_pipe_createPipe(settings);
12311231
ctx->currentXOR = 0x0d;
1232+
ctx->commandXOR = 0x0d;
12321233

12331234
phev_pipe_updateRegisterWithCallback(ctx, KO_WF_H_LAMP_CONT_SP, 1,(phev_pipe_updateRegisterCallback_t) test_phev_pipe_update_register_callback,NULL);
12341235

test/test_phev_service.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -944,7 +944,7 @@ void test_phev_service_end_to_end_operations(void)
944944
{
945945
const char * commands = "{ \"requests\": [{ \"operation\" : { \"airCon\" : \"on\" } }, { \"operation\" : { \"headLights\" : \"off\" } } ] }";
946946

947-
const uint8_t expected[] = {0xf6,0x04,0x00,0x04,0x02,0x00,0xf6,0x04,0x00,0x0a,0x02,0x06};
947+
const uint8_t expected_headlights_off[] = {0xf6,0x04,0x00,0x0a,0x02,0x06};
948948

949949
test_phev_service_global_in_in_message = msg_utils_createMsg(commands, strlen(commands));
950950

@@ -964,7 +964,7 @@ void test_phev_service_end_to_end_operations(void)
964964

965965
phev_pipe_loop(ctx->pipe);
966966
TEST_ASSERT_NOT_NULL(test_phev_service_global_out_out_message);
967-
TEST_ASSERT_EQUAL_MEMORY(expected, test_phev_service_global_out_out_message->data,sizeof(expected));
967+
TEST_ASSERT_EQUAL_MEMORY(expected_headlights_off, test_phev_service_global_out_out_message->data,sizeof(expected_headlights_off));
968968

969969
}
970970
void test_phev_service_end_to_end_updated_register(void)

0 commit comments

Comments
 (0)