Skip to content

Robusteres Parsing von Termux-Befehlen (quote‑tolerant)#120

Open
Android-PowerUser wants to merge 5 commits into
update-takescreenshot-to-use-completedfrom
improve-commandparser-robustness
Open

Robusteres Parsing von Termux-Befehlen (quote‑tolerant)#120
Android-PowerUser wants to merge 5 commits into
update-takescreenshot-to-use-completedfrom
improve-commandparser-robustness

Conversation

@Android-PowerUser

Copy link
Copy Markdown
Owner

Motivation

  • Die Erkennung von 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

  • Ersetzte das Termux(...)-Pattern in app/src/main/kotlin/com/google/ai/sample/util/CommandParser.kt durch ein quote‑tolerantes Regex, das den äußeren Quote‑Typ verwendet und verschachtelte Quotes im inneren Befehl erlaubt.
  • Passte die Extraktionsgruppe an (groupValues[2]) um den vollständigen, unveränderten Termux‑Befehl zu erhalten.
  • Ergänzte zwei Regressionstests in app/src/test/java/com/google/ai/sample/util/CommandParserTest.kt, die Termux("su -c 'ifconfig'") und Termux('su -c "ifconfig"') abdecken.

Testing

  • Kompiliert mit ./gradlew :app:compileDebugKotlin und der Build war erfolgreich.

Codex Task

@amazon-q-developer amazon-q-developer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛑 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.

Suggested change
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

  1. CWE-1333: Inefficient Regular Expression Complexity - https://cwe.mitre.org/data/definitions/1333.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant