Ensure all processed strings are null-terminated for safety#16
Merged
Conversation
Three changes: 1. ok_json.c: add upper-bound guard to the key_len counting loop in okj_find_value_index(). Previously the loop scanned until '\0' with no limit, so a caller passing a non-null-terminated key buffer would cause an out-of-bounds read. The loop now stops at OKJ_MAX_STRING_LEN + 1, which is one byte beyond the longest value any stored token can hold, so lookup behaviour is identical for valid inputs while overlong keys are safely rejected (returned length can never match a stored token). 2. ok_json.h / ok_json.c: add okj_copy_string() — a safe helper that copies an OkJsonString value into a caller-supplied buffer and always writes a null terminator. Copies at most buf_size-1 content bytes, then appends '\0'. Returns the number of characters copied, or 0 on bad arguments (NULL pointers, buf_size == 0). This fills the gap where callers previously had to perform the copy-and-terminate dance themselves, with no library-provided safety net. 3. test/ok_json_tests.c: add six new tests covering the above: - test_copy_string_basic — happy-path copy + content check - test_copy_string_null_terminated — explicit null-terminator check - test_copy_string_truncation — buffer smaller than string - test_copy_string_exact_fit — buffer exactly length+1 bytes - test_copy_string_null_inputs — NULL str, NULL buf, buf_size==0 - test_find_key_over_max_len — key > OKJ_MAX_STRING_LEN returns NULL All 56 tests pass with -Wall -Wextra -Werror -pedantic. https://claude.ai/code/session_01RYgCNpDtJiR6nMTBXtGbgR
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three changes:
ok_json.c: add upper-bound guard to the key_len counting loop in okj_find_value_index(). Previously the loop scanned until '\0' with no limit, so a caller passing a non-null-terminated key buffer would cause an out-of-bounds read. The loop now stops at OKJ_MAX_STRING_LEN + 1, which is one byte beyond the longest value any stored token can hold, so lookup behaviour is identical for valid inputs while overlong keys are safely rejected (returned length can never match a stored token).
ok_json.h / ok_json.c: add okj_copy_string() — a safe helper that copies an OkJsonString value into a caller-supplied buffer and always writes a null terminator. Copies at most buf_size-1 content bytes, then appends '\0'. Returns the number of characters copied, or 0 on bad arguments (NULL pointers, buf_size == 0). This fills the gap where callers previously had to perform the copy-and-terminate dance themselves, with no library-provided safety net.
test/ok_json_tests.c: add six new tests covering the above:
All 56 tests pass with -Wall -Wextra -Werror -pedantic.
https://claude.ai/code/session_01RYgCNpDtJiR6nMTBXtGbgR