Remote file inclusion from user supplied input

[GoogleCodeExporter]

Our security team has found several flaws that could allow users to potentially 
execute PHP remote file inclusion attacks.

The most common instances are where the PHP $_SERVER, $_GET, and $_REQUEST 
arrays are accessed. In these instances, they are not sanitized for input. As a 
result, later methods include files composed of these variables. 

This is most commonly found in 
* lib/SimpleSAML/Utilities.php (selfURLNoQuery, selfURL, and selfURLhost 
methods)

Other examples include:
* www/module.php: line 135
* www/shib13/sp/metadata.php: line 21
* www/saml2/sp/metadata.php: line 21
* www/saml2/sp/initSSO.php: lines 31-32

Original issue reported on code.google.com by kevin.w...@thomsonreuters.com on 13 Nov 2013 at 8:43

[GoogleCodeExporter]

Hi Kevin,

First of all, thanks for taking the time to file an issue.

Could you please provide all the examples that you have, if possible, with 
attack vector that confirm the vulnerabilities you are talking about.

As far as I can see, there's absolutely no vulnerability in the examples you 
are pointing out. Some of them just gather the entity ID from the request 
parameters in order to search for it in the configuration files afterwards. 
Others are indeed using the input to build the path to a file that will be 
included (module.php), but in that case, the input is properly sanitized before 
using it.

So unless you have more examples or can provide ways to exploit the code you 
are pointing to, I see no issue.

Original comment by jaim...@gmail.com on 15 Nov 2013 at 10:57

• Changed state: NeedsInfo

[GoogleCodeExporter]

Hi Jaime,

I see you point regarding the input sanitizing in module.php. I have requested 
a readout from our security team and full call stacks for each identified flaw.

Original comment by kevin.w...@thomsonreuters.com on 19 Nov 2013 at 1:47

[GoogleCodeExporter]

Thanks Kevin, let us know when you have some more details.

Original comment by jaim...@gmail.com on 22 Nov 2013 at 10:53

[GoogleCodeExporter]

Hi Jaime,

The call stacks weren't very useful. So I have gone through each identified 
high-severity flaw as best I can. Of the 14 identified, I found 2 that were 
actually of some concern:

1. demo.php, line 41 (/modules/oauth/bin/)
- I know this is a demo but the incoming $baseurl is not sanitized in any way. 
We can't assume this hasn't been manipulated in some way. This is considered a 
flaw based on it's use with the system function. 
- It's not as critical to sanitize the key and secret (though it would be 
good), but all variables should be encoded before displaying to the user.

2. ARP.php, line 31 (/modules/aggregator/lib/)
- There is no validation of the incoming $attributemap variable. As a result, 
incoming data from /modules/aggregator/www/arp.php (lines 37 & 44) pass data 
from the $_REQUEST array for logout link text.

Original comment by kevin.w...@thomsonreuters.com on 22 Nov 2013 at 2:53

[GoogleCodeExporter]

Thanks for the update Kevin!

I've been looking into both of them, and I think there's no big issue. Let me 
explain more in detail each one:

1. demo.php is a file intended for command-line-interface. It is not accessible 
via the web, nor by default (the module is disabled by default). There's no 
point to sanitize the $baseurl as the final URL is composed with it 
concatenated with more strings. Therefore, I don't see a way to leverage this 
to gain access to restricted files / URLs. Regarding key and secret, both would 
be more problematic if the output of the script were to be interpreted by a web 
browser (i.e.: XSS attacks), but in this case the output will be sent directly 
to the shell, so again, I don't see a way to leverage the lack of input 
sanitization here.

2. ARP.php. In this one you are right, there's no input sanitization at all. 
But the attributemap input variable is only used to build the full path to a 
PHP file. That means the only attack possible here would be to include a PHP 
file out of the attributemap directory, but that needs you to be able to put 
somewhere in the system a PHP file with the code you want to execute, so I 
consider this highly unlikely. Anyway, you are right and the input should be 
sanitized accordingly to remove any slashes and '..' on it.

Thanks again for reporting!

Original comment by jaim...@gmail.com on 23 Nov 2013 at 8:16

[GoogleCodeExporter]

The vulnerability in arp.php is now fixed in r3303.

Original comment by jaim...@gmail.com on 26 Nov 2013 at 3:13

• Changed state: Fixed

[GoogleCodeExporter]

Great! Thank you for such a quick response. 

Original comment by kevin.w...@thomsonreuters.com on 27 Nov 2013 at 4:43