Robusteres Parsing von Termux-Befehlen (quote‑tolerant)#120
Robusteres Parsing von Termux-Befehlen (quote‑tolerant)#120Android-PowerUser wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
Summary
This PR improves the Termux command parser to handle nested quotes, which is a valuable enhancement. However, there is a critical security vulnerability that must be addressed before merging.
Critical Issue
The new regex pattern contains nested quantifiers that create a catastrophic backtracking vulnerability (ReDoS). This could allow an attacker to cause the application to hang or crash with specially crafted input.
Required Action
Fix the ReDoS vulnerability in the regex pattern before merging. The suggested fix replaces the problematic nested quantifiers with a more efficient pattern that prevents exponential backtracking.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
| // Write text patterns | ||
| PatternInfo("writeText1", Regex("(?i)\\bwriteText\\([\"']([^\"']+)[\"']\\)"), { match -> Command.WriteText(match.groupValues[1]) }, CommandTypeEnum.WRITE_TEXT), | ||
| PatternInfo("termux1", Regex("(?i)\\bTermux\\([\"']([^\"']+)[\"']\\)"), { match -> Command.TermuxCommand(match.groupValues[1]) }, CommandTypeEnum.TERMUX_COMMAND), | ||
| PatternInfo("termux1", Regex("""(?i)\bTermux\(\s*(["'])((?:\\.|(?!\1\s*\)).)*)\1\s*\)"""), { match -> Command.TermuxCommand(match.groupValues[2]) }, CommandTypeEnum.TERMUX_COMMAND), |
There was a problem hiding this comment.
🛑 Security Vulnerability: This regex pattern is vulnerable to catastrophic backtracking (ReDoS attack)1. The nested quantifiers ((?:\\.|(?!\1\s*\)).)*) create exponential time complexity when the regex engine backtracks. An attacker could provide specially crafted input like Termux(" followed by thousands of characters without a closing quote, causing the application to hang or crash.
Replace with an atomic group or possessive quantifier to prevent backtracking. Use: ((?:\\.|(?!\1\s*\))[^\\])*) which matches non-backslash characters or escaped sequences without nested quantifiers.
| PatternInfo("termux1", Regex("""(?i)\bTermux\(\s*(["'])((?:\\.|(?!\1\s*\)).)*)\1\s*\)"""), { match -> Command.TermuxCommand(match.groupValues[2]) }, CommandTypeEnum.TERMUX_COMMAND), | |
| PatternInfo("termux1", Regex("""(?i)\bTermux\(\s*(["'])((?:\\.|(?!\1\s*\))[^\\])*)\1\s*\)"""), { match -> Command.TermuxCommand(match.groupValues[2]) }, CommandTypeEnum.TERMUX_COMMAND), |
Footnotes
-
CWE-1333: Inefficient Regular Expression Complexity - https://cwe.mitre.org/data/definitions/1333.html ↩
Motivation
Termux("...")schlug fehl, wenn der innere Befehl verschachtelte Quotes enthielt (z.B.Termux("su -c 'ifconfig'")), daher sollte der Parser robust gegenüber inneren Single-/Double‑Quotes werden.Description
Termux(...)-Pattern inapp/src/main/kotlin/com/google/ai/sample/util/CommandParser.ktdurch ein quote‑tolerantes Regex, das den äußeren Quote‑Typ verwendet und verschachtelte Quotes im inneren Befehl erlaubt.groupValues[2]) um den vollständigen, unveränderten Termux‑Befehl zu erhalten.app/src/test/java/com/google/ai/sample/util/CommandParserTest.kt, dieTermux("su -c 'ifconfig'")undTermux('su -c "ifconfig"')abdecken.Testing
./gradlew :app:compileDebugKotlinund der Build war erfolgreich.Codex Task