Skip to content

Fixes multiple memory safety bugs exposed by -fsanitize=address,undefined.#167

Merged
leo-aa88 merged 2 commits into
mainfrom
fix/issue-#162
Apr 3, 2026
Merged

Fixes multiple memory safety bugs exposed by -fsanitize=address,undefined.#167
leo-aa88 merged 2 commits into
mainfrom
fix/issue-#162

Conversation

@leo-aa88

@leo-aa88 leo-aa88 commented Apr 3, 2026

Copy link
Copy Markdown
Member

Description

This PR fixes a series of memory safety bugs discovered after enabling AddressSanitizer and UBSan in the build (-fsanitize=address,undefined -fno-omit-frame-pointer -g). All bugs share a common root cause: raw char * strings being used without explicit length tracking, causing over-reads, invalid frees, and buffer overflows. The fix systematically replaces unsafe patterns with the existing String struct ({ char *data; size_t len; }).


Makefile Change

Added sanitizer flags to CFLAGS for development builds:

# Before
CFLAGS := -Wall -Wextra -Wpedantic -Werror -O2 -Wuninitialized

# After
CFLAGS := -Wall -Wextra -Wpedantic -Werror -O2 -Wuninitialized -fsanitize=address,undefined -fno-omit-frame-pointer -g

Note: $(STDROT_LIB) is compiled without sanitizer flags since it's a shared library loaded at runtime via dlopen. If ASan is added to the .so build it must match the main binary's runtime exactly.


Bugs Fixed

semantic_analyzer.csafe_strdup called with len = MAX_BUFFER_LEN on string literals**

add_semantic_error constructed a String with .len = MAX_BUFFER_LEN regardless of actual message length, causing safe_strdup to memcpy 512 bytes off the end of short string literals into adjacent global memory. Fixed by changing add_semantic_error to accept a String parameter with correct length. Call sites using snprintf now capture the return value as the length. Bare string literal call sites use the new STRING_LITERAL(s) macro (sizeof(s) - 1).


New Macro Added (string_value.h)

#define STRING_LITERAL(s) ((String){ .data = (s), .len = sizeof(s) - 1 })

Safely wraps compile-time string literals into String structs with exact length computed at compile time — no strlen, no runtime cost.

Related Issue

Fixes #162

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Performance improvement
  • Refactor

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have documented my changes in the code or documentation
  • I have added tests that prove my changes work (if applicable)
  • I have run the unit tests locally
  • I have run the valgrind memory tests locally
  • All new and existing tests pass

@leo-aa88 leo-aa88 requested a review from SIGMazer as a code owner April 3, 2026 04:35
@leo-aa88 leo-aa88 self-assigned this Apr 3, 2026
@leo-aa88 leo-aa88 moved this to In Progress in Brainrot 0.0.1 Apr 3, 2026
@leo-aa88 leo-aa88 added bug Something isn't working refactor Modify existing working code but keeping the same functionality. CI Continuous integration labels Apr 3, 2026
@leo-aa88 leo-aa88 merged commit a967958 into main Apr 3, 2026
2 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Brainrot 0.0.1 Apr 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working CI Continuous integration refactor Modify existing working code but keeping the same functionality.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Add ASan + UBSan to CI

1 participant