Removed a potential memory leak in the GadgetBridge parser state machine

This commit is contained in:
Th3maz1ng 2023-10-15 11:02:55 +02:00
parent 5b0dea4cd7
commit 5cd5080fc9
5 changed files with 32 additions and 27 deletions

View File

@ -5,6 +5,7 @@
* over BLE. * over BLE.
* @version 0.1 * @version 0.1
* @date 2023-04-05 * @date 2023-04-05
* Updated : 2023-10-15, fixed potential memory leak.
* *
* @copyright MIT * @copyright MIT
* *
@ -254,6 +255,10 @@ gadget_bridge_parser_code_e gadget_bridge_parser_run(void)
switch(_gadget_bridge_internals.gadget_bridge_parser_fsm) switch(_gadget_bridge_internals.gadget_bridge_parser_fsm)
{ {
case GADGET_BRIDGE_PARSER_FSM_NEW_MESSAGE: case GADGET_BRIDGE_PARSER_FSM_NEW_MESSAGE:
// To prevent a potential memory leak if the parser is fed with bad data
_free_event_data();
if((start = strstr(_gadget_bridge_internals.buffer, "setTime(")) if((start = strstr(_gadget_bridge_internals.buffer, "setTime("))
&& (end = strstr(_gadget_bridge_internals.buffer, ");"))) && (end = strstr(_gadget_bridge_internals.buffer, ");")))
{ {
@ -1501,29 +1506,24 @@ static void _free_event_data(void)
{ {
case GADGET_BRIDGE_EVENT_TYPE_NOTIFY: case GADGET_BRIDGE_EVENT_TYPE_NOTIFY:
tls_mem_free(_gadget_bridge_internals.event_data.notification.title); tls_mem_free(_gadget_bridge_internals.event_data.notification.title);
_gadget_bridge_internals.event_data.notification.title = NULL;
tls_mem_free(_gadget_bridge_internals.event_data.notification.body); tls_mem_free(_gadget_bridge_internals.event_data.notification.body);
_gadget_bridge_internals.event_data.notification.body = NULL;
break; break;
case GADGET_BRIDGE_EVENT_TYPE_CALL: case GADGET_BRIDGE_EVENT_TYPE_CALL:
tls_mem_free(_gadget_bridge_internals.event_data.call.phone_number); tls_mem_free(_gadget_bridge_internals.event_data.call.phone_number);
_gadget_bridge_internals.event_data.call.phone_number = NULL;
tls_mem_free(_gadget_bridge_internals.event_data.call.contact); tls_mem_free(_gadget_bridge_internals.event_data.call.contact);
_gadget_bridge_internals.event_data.call.contact = NULL;
break; break;
case GADGET_BRIDGE_EVENT_TYPE_WEATHER: case GADGET_BRIDGE_EVENT_TYPE_WEATHER:
tls_mem_free(_gadget_bridge_internals.event_data.weather.location); tls_mem_free(_gadget_bridge_internals.event_data.weather.location);
_gadget_bridge_internals.event_data.weather.location = NULL;
tls_mem_free(_gadget_bridge_internals.event_data.weather.weather_desc); tls_mem_free(_gadget_bridge_internals.event_data.weather.weather_desc);
_gadget_bridge_internals.event_data.weather.weather_desc = NULL;
break; break;
case GADGET_BRIDGE_EVENT_TYPE_MUSIC_INFO: case GADGET_BRIDGE_EVENT_TYPE_MUSIC_INFO:
tls_mem_free(_gadget_bridge_internals.event_data.music_info.artist); tls_mem_free(_gadget_bridge_internals.event_data.music_info.artist);
_gadget_bridge_internals.event_data.music_info.artist = NULL;
tls_mem_free(_gadget_bridge_internals.event_data.music_info.track); tls_mem_free(_gadget_bridge_internals.event_data.music_info.track);
_gadget_bridge_internals.event_data.music_info.track = NULL;
break; break;
default: default:
break; return;
} }
// Since we freed any potential event, set the new type as none by setting all fields to 0
// this also ensures that pointers are set to NULL.
memset(&_gadget_bridge_internals.event_data, 0, sizeof _gadget_bridge_internals.event_data);
} }

View File

@ -5,6 +5,7 @@
* over BLE. * over BLE.
* @version 0.1 * @version 0.1
* @date 2023-04-05 * @date 2023-04-05
* Updated : 2023-10-15, fixed potential memory leak.
* *
* @copyright MIT * @copyright MIT
* *
@ -14,6 +15,7 @@
#include <stdio.h> #include <stdio.h>
#include <stdlib.h> #include <stdlib.h>
#include <string.h> #include <string.h>
#include <assert.h>
//#include "ble_service.h" //#include "ble_service.h"
bool ble_service_send_nus_data(const uint8_t *data, uint16_t length) bool ble_service_send_nus_data(const uint8_t *data, uint16_t length)
@ -249,6 +251,10 @@ gadget_bridge_parser_code_e gadget_bridge_parser_run(void)
switch(_gadget_bridge_internals.gadget_bridge_parser_fsm) switch(_gadget_bridge_internals.gadget_bridge_parser_fsm)
{ {
case GADGET_BRIDGE_PARSER_FSM_NEW_MESSAGE: case GADGET_BRIDGE_PARSER_FSM_NEW_MESSAGE:
// To prevent a potential memory leak if the parser is fed with bad data
_free_event_data();
if((start = strstr(_gadget_bridge_internals.buffer, "setTime(")) if((start = strstr(_gadget_bridge_internals.buffer, "setTime("))
&& (end = strstr(_gadget_bridge_internals.buffer, ");"))) && (end = strstr(_gadget_bridge_internals.buffer, ");")))
{ {
@ -1493,29 +1499,24 @@ static void _free_event_data(void)
{ {
case GADGET_BRIDGE_EVENT_TYPE_NOTIFY: case GADGET_BRIDGE_EVENT_TYPE_NOTIFY:
free(_gadget_bridge_internals.event_data.notification.title); free(_gadget_bridge_internals.event_data.notification.title);
_gadget_bridge_internals.event_data.notification.title = NULL;
free(_gadget_bridge_internals.event_data.notification.body); free(_gadget_bridge_internals.event_data.notification.body);
_gadget_bridge_internals.event_data.notification.body = NULL;
break; break;
case GADGET_BRIDGE_EVENT_TYPE_CALL: case GADGET_BRIDGE_EVENT_TYPE_CALL:
free(_gadget_bridge_internals.event_data.call.phone_number); free(_gadget_bridge_internals.event_data.call.phone_number);
_gadget_bridge_internals.event_data.call.phone_number = NULL;
free(_gadget_bridge_internals.event_data.call.contact); free(_gadget_bridge_internals.event_data.call.contact);
_gadget_bridge_internals.event_data.call.contact = NULL;
break; break;
case GADGET_BRIDGE_EVENT_TYPE_WEATHER: case GADGET_BRIDGE_EVENT_TYPE_WEATHER:
free(_gadget_bridge_internals.event_data.weather.location); free(_gadget_bridge_internals.event_data.weather.location);
_gadget_bridge_internals.event_data.weather.location = NULL;
free(_gadget_bridge_internals.event_data.weather.weather_desc); free(_gadget_bridge_internals.event_data.weather.weather_desc);
_gadget_bridge_internals.event_data.weather.weather_desc = NULL;
break; break;
case GADGET_BRIDGE_EVENT_TYPE_MUSIC_INFO: case GADGET_BRIDGE_EVENT_TYPE_MUSIC_INFO:
free(_gadget_bridge_internals.event_data.music_info.artist); free(_gadget_bridge_internals.event_data.music_info.artist);
_gadget_bridge_internals.event_data.music_info.artist = NULL;
free(_gadget_bridge_internals.event_data.music_info.track); free(_gadget_bridge_internals.event_data.music_info.track);
_gadget_bridge_internals.event_data.music_info.track = NULL;
break; break;
default: default:
break; return;
} }
// Since we freed any potential event, set the new type as none by setting all fields to 0
// this also ensures that pointers are set to NULL.
memset(&_gadget_bridge_internals.event_data, 0, sizeof _gadget_bridge_internals.event_data);
} }

View File

@ -16,18 +16,18 @@
<stdbool.h> <stdbool.h>
<time.h> <time.h>
1684074204 source:d:\users\think\programmation\arduino\git_projects\w800_smart_watch\src\gadget_bridge_parser\gadget_bridge.c 1697318967 source:d:\users\think\programmation\arduino\git_projects\w800_smart_watch\src\gadget_bridge_parser\gadget_bridge.c
"gadget_bridge.h" "gadget_bridge.h"
<stdio.h> <stdio.h>
<stdlib.h> <stdlib.h>
<string.h> <string.h>
1684074112 d:\users\think\programmation\arduino\git_projects\w800_smart_watch\src\gadget_bridge_parser\gadget_bridge.h 1684086410 d:\users\think\programmation\arduino\git_projects\w800_smart_watch\src\gadget_bridge_parser\gadget_bridge.h
<stdint.h> <stdint.h>
<stdbool.h> <stdbool.h>
<time.h> <time.h>
1684074076 source:d:\users\think\programmation\arduino\git_projects\w800_smart_watch\src\gadget_bridge_parser\main.c 1697358815 source:d:\users\think\programmation\arduino\git_projects\w800_smart_watch\src\gadget_bridge_parser\main.c
<stdio.h> <stdio.h>
<stdlib.h> <stdlib.h>
<string.h> <string.h>

View File

@ -2,19 +2,19 @@
<CodeBlocks_layout_file> <CodeBlocks_layout_file>
<FileVersion major="1" minor="0" /> <FileVersion major="1" minor="0" />
<ActiveTarget name="Debug" /> <ActiveTarget name="Debug" />
<File name="main.c" open="0" top="0" tabpos="1" split="0" active="1" splitpos="0" zoom_1="0" zoom_2="0"> <File name="gadget_bridge.h" open="1" top="0" tabpos="2" split="0" active="1" splitpos="0" zoom_1="0" zoom_2="0">
<Cursor> <Cursor>
<Cursor1 position="11119" topLine="337" /> <Cursor1 position="4496" topLine="288" />
</Cursor> </Cursor>
</File> </File>
<File name="gadget_bridge.h" open="1" top="0" tabpos="13" split="0" active="1" splitpos="0" zoom_1="0" zoom_2="0"> <File name="gadget_bridge.c" open="1" top="1" tabpos="1" split="0" active="1" splitpos="0" zoom_1="0" zoom_2="0">
<Cursor> <Cursor>
<Cursor1 position="4063" topLine="0" /> <Cursor1 position="71209" topLine="1490" />
</Cursor> </Cursor>
</File> </File>
<File name="gadget_bridge.c" open="1" top="1" tabpos="12" split="0" active="1" splitpos="0" zoom_1="0" zoom_2="0"> <File name="main.c" open="1" top="0" tabpos="3" split="0" active="1" splitpos="0" zoom_1="0" zoom_2="0">
<Cursor> <Cursor>
<Cursor1 position="2213" topLine="94" /> <Cursor1 position="11686" topLine="156" />
</Cursor> </Cursor>
</File> </File>
</CodeBlocks_layout_file> </CodeBlocks_layout_file>

View File

@ -320,6 +320,10 @@ const char *sample[] =
"ith a very long content ", "ith a very long content ",
"to make sure this case i", "to make sure this case i",
"s handled in the parser ", "s handled in the parser ",
// Let's simulate bad data to check for robustness
"[16]GB({t:\"notify\",id:",
"and we do not crash the ", "and we do not crash the ",
"thing because we forgot ", "thing because we forgot ",
"to handle such a case, d", "to handle such a case, d",