From 7b7b84eb68fbdbc77eb31768490c276a5e463aae Mon Sep 17 00:00:00 2001 From: Zach Borboa Date: Wed, 12 Oct 2016 23:43:24 -0700 Subject: [PATCH] Add additional security considerations --- README.md | 2 +- SECURITY.md | 65 ++++++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 61 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 6deb6d1..b77fe42 100644 --- a/README.md +++ b/README.md @@ -282,7 +282,7 @@ MultiCurl::verbose($on = true, $output = STDERR) ### Security -See [SECURITY.md](https://github.com/php-curl-class/php-curl-class/blob/master/SECURITY.md). +See [SECURITY](https://github.com/php-curl-class/php-curl-class/blob/master/SECURITY.md) for security considerations. ### Contribute 1. Check for open issues or open a new issue to start a discussion around a bug or feature. diff --git a/SECURITY.md b/SECURITY.md index 61a5e98..18c1305 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -1,11 +1,66 @@ -### Security +### Security Considerations -* Don't blindly accept arbitrary urls. Curl supports many protocols including `FILE`. The following would show the contents of `file:///etc/passwd`. +* Url may point to system files. Don't blindly accept arbitrary urls from users. Curl supports many protocols including + `FILE`. The following would show the contents of `file:///etc/passwd`. + +```bash +# Attacker. +$ curl https://www.example.com/display_webpage.php?url=file%3A%2F%2F%2Fetc%2Fpasswd +`` ```php -// https://www.example.com/fetch_page.php?url=file%3A%2F%2F%2Fetc%2Fpasswd -$unsafe_url = $_GET['url']; // DANGER! +// display_webpage.php +$url = $_GET['url']; // DANGER! $curl = new Curl(); -$curl->get($unsafe_url); +$curl->get($url); echo $curl->response; ``` + +Safer: + +```php +function is_website_url($url, $allowed_schemes = array('http', 'https')) { + $validate_url = !(filter_var($url, FILTER_VALIDATE_URL) === false); + $scheme = parse_url($url, PHP_URL_SCHEME); + return $validate_url && in_array($scheme, $allowed_schemes, true); +} + +$url = $_GET['url']; +if (!is_website_url($url)) { + die('Unsafe url detected.'); +} +``` + +* Url may point to internal urls behind firewall (e.g. http://192.168.0.1/ or ftp://192.168.0.1/). Use a whitelist to + allow certain urls. Definitely don't use a blacklist. + +* Request data prefixed with the @ character may have special interpretation and read from system files. + +```bash +# Attacker. +$ curl https://www.example.com/upload_photo.php --data "photo=@/etc/password" +`` + +```php +// upload_photo.php +$curl = new Curl(); +$curl->post('http://www.anotherwebsite.com/', array( + 'photo' => $_POST['photo'], // DANGER! +)); +``` + +* Requests with redirection enabled may return responses from unexpected sources. + Downloading https://www.example.com/image.png may redirect and download https://www.evil.com/virus.exe + +```php +$curl = new Curl(); +$curl->setOpt(CURLOPT_FOLLOWLOCATION, true); // DANGER! +$curl->download('http://www.example.com/image.png', 'my_image.png'); +``` + +* Do not disable SSL protections. + +```php +curl_setopt($ch, CURLOPT_SSL_VERIFYHOST, FALSE); // DANGER! +curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, FALSE); // DANGER! +```