From 08b9f20df1c108be5c1ab8b32c0fcbed31a079b3 Mon Sep 17 00:00:00 2001 From: medhavini Date: Mon, 20 Jul 2015 20:35:39 +0530 Subject: Security check fail Security check condition to check that the path is NOT a URL may give false negative in case of subdomains. Where URLs don't start with http or www. --- system/helpers/path_helper.php | 1 + 1 file changed, 1 insertion(+) (limited to 'system') diff --git a/system/helpers/path_helper.php b/system/helpers/path_helper.php index c23ec6435..34eebc4b0 100644 --- a/system/helpers/path_helper.php +++ b/system/helpers/path_helper.php @@ -61,6 +61,7 @@ if ( ! function_exists('set_realpath')) function set_realpath($path, $check_existance = FALSE) { // Security check to make sure the path is NOT a URL. No remote file inclusion! + // PROBLEM HERE - this can be easily bypassed in case of subdomains if (preg_match('#^(http:\/\/|https:\/\/|www\.|ftp|[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3})#i', $path)) { show_error('The path you submitted must be a local server path, not a URL'); -- cgit v1.2.3-24-g4f1b From 78e1b70e35b45455728e4126ed1b19d6332ad26b Mon Sep 17 00:00:00 2001 From: rajatsharma94 Date: Mon, 20 Jul 2015 22:49:56 +0530 Subject: Failed security check The implemented security check to make sure the path is NOT a URL can easily be bypassed (gives false negative) for all subdomains. Eg "subdomain.domain.com" should ideally show an error but it does not. The new security check tries to make a fsockopen connection to validate whether the URL is external or not. --- system/helpers/path_helper.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'system') diff --git a/system/helpers/path_helper.php b/system/helpers/path_helper.php index 34eebc4b0..019e220f3 100644 --- a/system/helpers/path_helper.php +++ b/system/helpers/path_helper.php @@ -61,8 +61,7 @@ if ( ! function_exists('set_realpath')) function set_realpath($path, $check_existance = FALSE) { // Security check to make sure the path is NOT a URL. No remote file inclusion! - // PROBLEM HERE - this can be easily bypassed in case of subdomains - if (preg_match('#^(http:\/\/|https:\/\/|www\.|ftp|[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3})#i', $path)) + if (preg_match('#^(http:\/\/|https:\/\/|www\.|ftp|[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3})#i', $path) || ( function_exists('fsockopen') && @fsockopen($path, 80, $errno, $errstr, 30))) { show_error('The path you submitted must be a local server path, not a URL'); } -- cgit v1.2.3-24-g4f1b From 5545dcdc170eca21c3d2c91e10556698f9512643 Mon Sep 17 00:00:00 2001 From: medhavini Date: Thu, 23 Jul 2015 18:59:44 +0530 Subject: IP Address checking generates false positives. IP Address checking marks all IPs between 0.0.0.0 - 999.999.999.999 as valid IP Address. Which is not true. --- system/helpers/path_helper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'system') diff --git a/system/helpers/path_helper.php b/system/helpers/path_helper.php index 019e220f3..cf6be776f 100644 --- a/system/helpers/path_helper.php +++ b/system/helpers/path_helper.php @@ -61,7 +61,7 @@ if ( ! function_exists('set_realpath')) function set_realpath($path, $check_existance = FALSE) { // Security check to make sure the path is NOT a URL. No remote file inclusion! - if (preg_match('#^(http:\/\/|https:\/\/|www\.|ftp|[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3})#i', $path) || ( function_exists('fsockopen') && @fsockopen($path, 80, $errno, $errstr, 30))) + if (preg_match('#^(http:\/\/|https:\/\/|www\.|ftp)#i', $path)) { show_error('The path you submitted must be a local server path, not a URL'); } -- cgit v1.2.3-24-g4f1b From d2f63a1803503a09b2d92d4aedd793636d470c7b Mon Sep 17 00:00:00 2001 From: rajatsharma94 Date: Thu, 23 Jul 2015 19:05:17 +0530 Subject: IP checking false positives and no ipv6 check The currently implemented method marks all IPs between 0.0.0.0 - 999.999.999.999 as valid IP Address. Which generates false positives as any IP after 255.255.255.255 is not a valid IP address. Also, there is no check for IPv6 IP addresses. filter_var() solves both the issues. --- system/helpers/path_helper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'system') diff --git a/system/helpers/path_helper.php b/system/helpers/path_helper.php index cf6be776f..dbe090058 100644 --- a/system/helpers/path_helper.php +++ b/system/helpers/path_helper.php @@ -61,7 +61,7 @@ if ( ! function_exists('set_realpath')) function set_realpath($path, $check_existance = FALSE) { // Security check to make sure the path is NOT a URL. No remote file inclusion! - if (preg_match('#^(http:\/\/|https:\/\/|www\.|ftp)#i', $path)) + if (preg_match('#^(http:\/\/|https:\/\/|www\.|ftp)#i', $path) || (!filter_var($ip, FILTER_VALIDATE_IP) === false)) { show_error('The path you submitted must be a local server path, not a URL'); } -- cgit v1.2.3-24-g4f1b From d41ec5c4e225aaa766a8b9e2856a180cf85cb343 Mon Sep 17 00:00:00 2001 From: rajatsharma94 Date: Thu, 23 Jul 2015 19:59:50 +0530 Subject: Update path_helper.php --- system/helpers/path_helper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'system') diff --git a/system/helpers/path_helper.php b/system/helpers/path_helper.php index dbe090058..b8640d2e4 100644 --- a/system/helpers/path_helper.php +++ b/system/helpers/path_helper.php @@ -61,7 +61,7 @@ if ( ! function_exists('set_realpath')) function set_realpath($path, $check_existance = FALSE) { // Security check to make sure the path is NOT a URL. No remote file inclusion! - if (preg_match('#^(http:\/\/|https:\/\/|www\.|ftp)#i', $path) || (!filter_var($ip, FILTER_VALIDATE_IP) === false)) + if (preg_match('#^(http:\/\/|https:\/\/|www\.|ftp)#i', $path) OR ( ! filter_var($path, FILTER_VALIDATE_IP) === false)) { show_error('The path you submitted must be a local server path, not a URL'); } -- cgit v1.2.3-24-g4f1b From 591166310919d1f46facf9e6b829d4dcf399bab6 Mon Sep 17 00:00:00 2001 From: rajatsharma94 Date: Thu, 23 Jul 2015 20:02:13 +0530 Subject: Security check updated. All security check conditions are modified according to CI styleguide. --- system/helpers/path_helper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'system') diff --git a/system/helpers/path_helper.php b/system/helpers/path_helper.php index b8640d2e4..3e5369195 100644 --- a/system/helpers/path_helper.php +++ b/system/helpers/path_helper.php @@ -61,7 +61,7 @@ if ( ! function_exists('set_realpath')) function set_realpath($path, $check_existance = FALSE) { // Security check to make sure the path is NOT a URL. No remote file inclusion! - if (preg_match('#^(http:\/\/|https:\/\/|www\.|ftp)#i', $path) OR ( ! filter_var($path, FILTER_VALIDATE_IP) === false)) + if (preg_match('#^(http:\/\/|https:\/\/|www\.|ftp)#i', $path) OR ( ! filter_var($path, FILTER_VALIDATE_IP) === FALSE)) { show_error('The path you submitted must be a local server path, not a URL'); } -- cgit v1.2.3-24-g4f1b From 08c1a111916a1740e7c33f11ed7097832e1b97df Mon Sep 17 00:00:00 2001 From: rajatsharma94 Date: Thu, 23 Jul 2015 20:31:05 +0530 Subject: Update path_helper.php --- system/helpers/path_helper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'system') diff --git a/system/helpers/path_helper.php b/system/helpers/path_helper.php index 3e5369195..c96d0b8b3 100644 --- a/system/helpers/path_helper.php +++ b/system/helpers/path_helper.php @@ -61,7 +61,7 @@ if ( ! function_exists('set_realpath')) function set_realpath($path, $check_existance = FALSE) { // Security check to make sure the path is NOT a URL. No remote file inclusion! - if (preg_match('#^(http:\/\/|https:\/\/|www\.|ftp)#i', $path) OR ( ! filter_var($path, FILTER_VALIDATE_IP) === FALSE)) + if (preg_match('#^(http:\/\/|https:\/\/|www\.|ftp)#i', $path) OR filter_var($path, FILTER_VALIDATE_IP) === $path ) { show_error('The path you submitted must be a local server path, not a URL'); } -- cgit v1.2.3-24-g4f1b