[GH-ISSUE #37] Improve character escaping #27

New issue

Closed

opened 2026-02-28 00:40:13 +03:00 by kerem · 1 comment

kerem commented

2026-02-28 00:40:13 +03:00

Owner

Originally created by @devicenull on GitHub (Jul 5, 2016).
Original GitHub issue: https://github.com/telephone/LookingGlass/issues/37

This method of escaping host IPs is pretty scary:

    // sanitize + remove single quotes
    $host = str_replace('\'', '', filter_var($host, FILTER_SANITIZE_URL));
    // execute command
    $process = proc_open("{$cmd} '{$host}'", $spec, $pipes, null);

Per the PHP docs:

FILTER_SANITIZE_URL -> Remove all characters except letters, digits and $-_.+!*'(),{}|^~[]`<>#%";/?:@&=.

The majority of those characters have no business being in a domain name/IP address. I'd suggest checking the host against FILTER_VALIDATE_IP instead... if that doesn't validate, pass it to gethostbyname and use whatever that returns.

Using escapeshellarg instead of just '{$host}' would also be a good move.

I don't see an obvious exploit for this, but that code really stands out to me as suspicious.

Originally created by @devicenull on GitHub (Jul 5, 2016). Original GitHub issue: https://github.com/telephone/LookingGlass/issues/37 This method of escaping host IPs is pretty scary: ``` // sanitize + remove single quotes $host = str_replace('\'', '', filter_var($host, FILTER_SANITIZE_URL)); // execute command $process = proc_open("{$cmd} '{$host}'", $spec, $pipes, null); ``` Per the PHP docs: FILTER_SANITIZE_URL -> Remove all characters except letters, digits and $-_.+!*'(),{}|\^~[]`<>#%";/?:@&=. The majority of those characters have no business being in a domain name/IP address. I'd suggest checking the host against `FILTER_VALIDATE_IP` instead... if that doesn't validate, pass it to gethostbyname and use whatever that returns. Using `escapeshellarg` instead of just `'{$host}'` would also be a good move. I don't see an obvious exploit for this, but that code really stands out to me as suspicious.

kerem

2026-02-28 00:40:13 +03:00

closed this issue
added the
v1
label

kerem commented

2026-02-28 00:40:13 +03:00

Author

Owner

@telephone commented on GitHub (Jul 7, 2016):

If you look at LookingGlass::validate() and it's subsidiary calls, you'll notice it's not that scary. The input is always validated against an IP or URL. The part that you've quoted (FILTER_SANITIZE_URL) is there as a backup.

Using escapeshellarg() isn't necessary, as I've already ensured single quotes are removed (in the section you've quoted). Instead of escaping single quotes via escapeshellarg(), I'm simply removing them.

@telephone commented on GitHub (Jul 7, 2016): If you look at `LookingGlass::validate()` and it's subsidiary calls, you'll notice it's not that scary. The input is always validated against an IP or URL. The part that you've quoted (`FILTER_SANITIZE_URL`) is there as a backup. Using `escapeshellarg()` isn't necessary, as I've already ensured single quotes are removed (in the section you've quoted). Instead of escaping single quotes via `escapeshellarg()`, I'm simply removing them.

No labels

enhancement

pull-request

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

starred/LookingGlass#27

No description provided.

Rows
Columns