Skip to content

Commit f834faa

Browse files
Copilotwannaphong
andcommitted
Additional security improvements: fix memory handling edge cases
Co-authored-by: wannaphong <8536487+wannaphong@users.noreply.github.com>
1 parent b0ca94d commit f834faa

File tree

3 files changed

+19
-5
lines changed

3 files changed

+19
-5
lines changed

_codeql_detected_source_root

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
.

src/trie.c

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,16 +83,23 @@ static TrieNode* trie_node_add_child(TrieNode* node, int codepoint) {
8383
if (node->num_children >= node->capacity) {
8484
int new_capacity = node->capacity == 0 ? INITIAL_CAPACITY : node->capacity * 2;
8585

86+
/* Allocate both arrays before updating pointers */
8687
TrieNode** new_children = (TrieNode**)realloc(node->children,
8788
new_capacity * sizeof(TrieNode*));
8889
if (!new_children) return NULL;
8990

9091
int* new_chars = (int*)realloc(node->child_chars, new_capacity * sizeof(int));
9192
if (!new_chars) {
92-
/* Restore old pointer to avoid leak, but still fail */
93+
/* new_children was allocated but new_chars failed */
94+
/* Since realloc succeeded for new_children, the old pointer is invalid */
95+
/* We must use the new_children pointer, even though we can't proceed */
96+
node->children = new_children;
97+
/* Alternatively, we could try to restore by reallocating to old size */
98+
/* But that could also fail, so we just update and return NULL */
9399
return NULL;
94100
}
95101

102+
/* Both allocations succeeded, update pointers */
96103
node->children = new_children;
97104
node->child_chars = new_chars;
98105
node->capacity = new_capacity;
@@ -201,8 +208,11 @@ int trie_prefixes(Trie* trie, const char* text, char*** prefixes, int** lengths)
201208
*prefixes = (char**)malloc(max_prefixes * sizeof(char*));
202209
*lengths = (int*)malloc(max_prefixes * sizeof(int));
203210
if (!*prefixes || !*lengths) {
204-
free(*prefixes);
205-
free(*lengths);
211+
/* Clean up any successful allocation */
212+
if (*prefixes) free(*prefixes);
213+
if (*lengths) free(*lengths);
214+
*prefixes = NULL;
215+
*lengths = NULL;
206216
return 0;
207217
}
208218

tests/test_newmm.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ typedef struct {
1717
static int test_count = 0;
1818
static int test_passed = 0;
1919

20+
#define INITIAL_OUTPUT_SIZE 1024
21+
#define TOKEN_OVERHEAD 4 /* For "'', " around each token */
22+
2023
void run_test(const char* text, const char* dict_path, const char* expected, const char* description) {
2124
test_count++;
2225
printf("\n[Test %d] %s\n", test_count, description);
@@ -44,7 +47,7 @@ void run_test(const char* text, const char* dict_path, const char* expected, con
4447
}
4548

4649
/* Build output string with dynamic allocation */
47-
size_t output_size = 1024;
50+
size_t output_size = INITIAL_OUTPUT_SIZE;
4851
char* output = (char*)malloc(output_size);
4952
if (!output) {
5053
printf("❌ FAIL: Memory allocation failed\n");
@@ -54,7 +57,7 @@ void run_test(const char* text, const char* dict_path, const char* expected, con
5457

5558
strcpy(output, "[");
5659
for (int i = 0; i < token_count; i++) {
57-
size_t needed = strlen(output) + strlen(tokens[i]) + 10; /* "'', " + margins */
60+
size_t needed = strlen(output) + strlen(tokens[i]) + TOKEN_OVERHEAD + 1; /* +1 for null */
5861
if (needed >= output_size) {
5962
output_size = needed * 2;
6063
char* new_output = (char*)realloc(output, output_size);

0 commit comments

Comments
 (0)