Fix locale bug in grep-find-unicode-wrapper BiDi character detection#36
Open
assisted-by-ai wants to merge 5 commits intoKicksecure:masterfrom
Open
Fix locale bug in grep-find-unicode-wrapper BiDi character detection#36assisted-by-ai wants to merge 5 commits intoKicksecure:masterfrom
assisted-by-ai wants to merge 5 commits intoKicksecure:masterfrom
Conversation
Tests 66 cases across 10 categories: clean files, ASCII control chars, BiDi/Trojan Source chars, invisible/zero-width chars, homoglyphs, Unicode spaces, tag characters, malformed UTF-8, sneaky embeddings, and edge cases. No actual bypass found - checks 1+2 catch all non-ASCII. Documents a locale bug: check 3's $'\uXXXX' expansion requires a UTF-8 locale. In non-UTF-8 locales the pattern degrades to literal chars causing false positives, though BiDi detection is still covered by checks 1+2. https://claude.ai/code/session_01726gqqGv3oaDV5jLbM6E6h
The wrapper's check 3 uses $'\uXXXX' which is expanded at bash parse time using the caller's locale - NOT the LC_ALL=C on the grep command. In non-UTF-8 locales, \u sequences pass through literally, creating a bracket expression of [0-9A-Fu\] that false-positives on files with digits, hex values, UUIDs, backslashes, or typical code. New tests verify this: 5/6 clean ASCII files trigger false positives in non-UTF-8 locales, 0/6 in UTF-8 locales. Fix: use \x byte sequences. https://claude.ai/code/session_01726gqqGv3oaDV5jLbM6E6h
- grep-find-unicode-wrapper: check 3 used $'\uXXXX' which requires a UTF-8 locale at bash parse time. In non-UTF-8 locales the pattern degrades to literal ASCII chars causing false positives. Fixed by using \x byte sequences. Old line kept commented out with explanation. - test script: use get_colors.sh instead of custom color vars, handle old-pattern false positives as expected warns not failures. - run-tests: call tests/test_grep_find_unicode_wrapper. https://claude.ai/code/session_01726gqqGv3oaDV5jLbM6E6h
…ventions - Use real grep-find-unicode-wrapper binary instead of reimplementing the four grep checks locally. Tests exercise actual code paths. - Source get_colors.sh for colors instead of custom color variables. - Create safe-rm-maybe.bsh providing rm-safe-maybe function that uses safe-rm if installed, otherwise falls back to rm. - Use long options (--recursive, --force, --directory, --delete, --address-radix, --format) instead of short flags. - Use `| tee -- "$file_name"` instead of `> "$file_name"` for better xtrace output and error handling. - Rename variable 'file' to 'file_name' to avoid collision with the standard unix 'file' utility. - Remove assumption that tools might not be installed; tests require all tools available (installed from source or on disk). https://claude.ai/code/session_01726gqqGv3oaDV5jLbM6E6h
- Use 'has' from has.sh instead of 'command -v'. - Use &>/dev/null instead of >/dev/null 2>&1. - Replace offensive/risky test examples (rm -rf, root escalation) with safe alternatives (GOOD/BADX overwrite, harmless error messages). - Move inline Python for long-line generation to separate script usr/libexec/helper-scripts/write-long-line-with-unicode. https://claude.ai/code/session_01726gqqGv3oaDV5jLbM6E6h
Contributor
|
Rejected, the test scripts are redundant with |
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
3 participants
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.
Summary
Fixed a critical locale-dependent bug in
grep-find-unicode-wrapperwhere Unicode BiDi character detection would fail in non-UTF-8 locales (LANG=C, empty LANG) and produce false positives on normal files containing digits or hex characters.Key Changes
Fixed BiDi character detection pattern: Replaced bash
$'\uXXXX'Unicode escape sequences with explicit\xbyte sequences in the grep pattern. The$'\uXXXX'syntax is expanded at bash parse time using the caller's locale, not theLC_ALL=Cset on the grep command. In non-UTF-8 locales, this caused the literal characters\uto be passed to grep, creating a character class[0-9A-Fu\]that matched almost any file with digits or hex values.Added comprehensive test suite: Created
tests/test_grep_find_unicode_wrapperwith 483 lines of tests covering:Added helper script: Created
usr/libexec/helper-scripts/safe-rm-maybe.bshto provide a safe file removal function that usessafe-rmif available, otherwise falls back torm.Updated test runner: Modified
run-teststo execute the new comprehensive test suite.Implementation Details
The fix converts the BiDi character detection from:
$'[\u061C\u200E\u200F\u202A\u202B\u202C\u202D\u202E\u2066\u2067\u2068\u2069]'To explicit UTF-8 byte sequences:
This ensures the pattern works correctly regardless of the caller's locale. BiDi characters are still caught by checks 1 and 2 (non-ASCII byte detection), so this is purely a false positive fix with no security bypass.
https://claude.ai/code/session_01726gqqGv3oaDV5jLbM6E6h