Skip to content

fix: prevent argument injection in OpenVPN config deletion#2086

Open
tranquac wants to merge 1 commit intoRaspAP:masterfrom
tranquac:fix/argument-injection-openvpn
Open

fix: prevent argument injection in OpenVPN config deletion#2086
tranquac wants to merge 1 commit intoRaspAP:masterfrom
tranquac:fix/argument-injection-openvpn

Conversation

@tranquac
Copy link
Copy Markdown

Summary

Fix argument injection vulnerability in OpenVPN config deletion endpoint that allows authenticated users to delete arbitrary files as root.

Problem

ajax/openvpn/del_ovpncfg.php uses escapeshellcmd() which does not prevent argument injection:

$ovpncfg_id = escapeshellcmd($_POST['cfg_id']);
$ovpncfg_files = pathinfo(...).'/'.$ovpncfg_id.'_*.conf';
exec("sudo rm $ovpncfg_files", $return);

Why escapeshellcmd is insufficient

escapeshellcmd() escapes shell metacharacters (;, |, &, etc.) but does NOT prevent:

  1. Argument injection: cfg_id=-rf /sudo rm /path/-rf /_*.conf — the -rf flag is interpreted by rm
  2. Path traversal: cfg_id=../../etc/important → deletes files outside the VPN config directory

Since this runs with sudo, an attacker with basic auth can delete any file on the system as root, leading to denial of service or privilege escalation.

escapeshellcmd vs escapeshellarg

Function Prevents Does NOT prevent
escapeshellcmd Command chaining (;, |, &) Argument/flag injection, path traversal
escapeshellarg All injection (wraps in single quotes)

Fix

  1. Validate cfg_id with allowlist regex: only [a-zA-Z0-9_-] characters
  2. Replace escapeshellcmd with escapeshellarg for proper argument escaping
  3. Escape the directory path separately

Impact

  • Type: Argument Injection / Arbitrary File Deletion as Root (CWE-88)
  • Affected endpoint: ajax/openvpn/del_ovpncfg.php
  • Risk: Delete any file on the system as root (DoS, privilege escalation)
  • OWASP: A03:2021 — Injection

Signed-off-by: tranquac <tranquac@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant