Failed to block .phps and .htaccess file upload in #REQUEST-933-APPLICATION-ATTACK-PHP rules

[umarfarook882]

When i was going through #REQUEST-933-APPLICATION-ATTACK-PHP #Rule ID:933110.
At first, i thought something was missing. I done my little research :)

PHP support Extension: .php, .phtml, .php3, .php4, .php5, .php7, .phps

then i found .phps extension was missing on the regex. so i was able to upload .phps file. Anyway it not a major bug, because by default on apache, it will not allow .phps file to run. it has no use. So i did find another way to execute the .phps file by uploading the .htaccess file.

Then i thought definitely OWASP CRS will block uploading .htacess file, but surprisingly there is no rule to block .htaccess file upload. Finally i uploaded .htaccess file and execute the .phps which gives shell access :)

So its better to block the .htaccess and .phps file upload for better security. :)

For more information & detailed explanation check my demo video on Github

[dune73]

Thanks for contributing this find @umarfarook882. We are very agnostic towards file uploads so far. The thinking is that the webserver should protect itself. But then if the webserver should be able to protect itself, what's the use of CRS?

So you are probably right, that we should add this. Other thoughts?

[lifeforms]

I would very much like to prevent .htaccess from being uploaded!

I'm not so very sure about .phps. I don't think it does anything special.

With a .htaccess you can make every file an executable PHP file, e.g.

<FilesMatch \.blah$>
	SetHandler application/x-httpd-php
</FilesMatch>

Then upload a test.blah file and execute it.

So let's add .htaccess. Only question is where? Rule 933110 is PHP specific. But .htaccess is not PHP specific. So maybe we should create a different general rule? I could see us adding more entries in the future.

[umarfarook882]

@lifeforms yes, i agree. we can allow any file extension to execute as application/x-httpd-php using .htaccess. but just for explaining the issue. i have used the .phps file extension :). Another reason why i point out .phps is because it is not included in regex. #REQUEST-933-APPLICATION-ATTACK-PHP #Rule ID:933110

.phps is also one of the extension supported by php

.htaccess is not related to PHP, better we should create separate rule to this issue:)

Check my demo video on Github, i have explained the issue clearly. :)

[victorhora]

+1 on trying blocking .htaccess upload.

Not sure what's the best approach here but I'm also unsure where such rule should go. I just glanced at this before going to bed, but maybe creating REQUEST-9XX-ATTACK-UNRESTRICTED-FILE-UPLOAD makes sense with something like SecRule FILES_NAMES|FILES "@pmf restricted-files.data" as a start testbed.

[dune73]

I think this is related to REQUEST-930-APPLICATION-ATTACK-LFI.conf so that's where I think the new rule belongs. Then a data file as suggested.

There might be FPs, so not sure if this should go in PL1 or better PL2. Maybe testing it a bit beforehand.

[victorhora]

Hummm I see your point @dune73. Actually I've thought about "restricted-files.data" based on rule 930130 because .htaccess is already there.

If this ends up going to PL1 and at REQUEST-930-APPLICATION-ATTACK-LFI.conf, maybe adding **FILES_NAMES and FILES variables to 930130 and see how it goes with FPs...

Negative side IMHO is mixing two slightly different vulnerability classes on a single rule (or file), but not adding a new rule can be positive...

[dune73]

Yes, I expected to end up with a file that would resemble that one - or re-using it straight away. ;)

Re-using an existing rule file that does not really fit needs to be balanced with creating a new rule file with very few rules - which is annoying for nginx users as they need to include all files explicitly.

However, we also have plans to invest into fuzzy-hash stuff aimed at file uploads. So maybe an independent rule file will have 1-2 additional rules. Hard to tell. :)

[umarfarook882]

@dune73 What about this issue?

As @victorhora suggestion, i checked it works goods.But also its kind of mixing two different vulnerability classes on a single rule and It will create confusion for users in future while analyzing modsecurity log.

so any idea for the issue? :)

[dune73]

Busy times, sorry for the late response @umarfarook882. I suggest we take this into the community chat tonight and decide on the course of action there. I take it, this will be the middle of the night for you, but there is no imminent need to take part - but you would be very welcome to participate of course.

[dune73]

So we talked this through in the chat. We agree with all your proposals @umarfarook882.

@lifeforms volunteered to do the PR. And it is likely to happen for the 3.0.3 release.

[lifeforms]

Add .phps checking to existing rule - v3.0.3

Add .htaccess checking in separate rule - v3.1.0