octobercms / october

Self-hosted CMS platform based on the Laravel PHP Framework.
https://octobercms.com/
Other
11.03k stars 2.21k forks source link

.htaccess files allowed in themes #3257

Closed d4wner closed 6 years ago

d4wner commented 6 years ago
Expected behavior

Well, dear sir, I just found an arbitrary upload vulnerability in Octobercms of the latest version.

ADLab of Venustech

Reproduce steps

When you login into the backend, you can visit: http://localhost/backend/cms/themes You can get the demo zip by export from demo theme, and now you have a zip file of demo. Then you can add two file into the zip file,one evil php file and one .htaccess file.

.htaccess

<IfModule mod_rewrite.c>

    ##
    ## Standard routes
    ##
    RewriteCond %{REQUEST_FILENAME} !-f
    RewriteRule ^ evil.php [L]

</IfModule>

evil.php

<?php 
phpinfo();
?>

Now, I should upload the modified zip file , and import to cover the original demo theme folder.

When I upload successful, I'll get an evil file here . Because of the new .htaccess file, I can visit the evil.php file directly now, I can see the php infomation of the server.

http://localhost/themes/demo/assets/evil.php

October build

The latest version.

Wish your response ,sir!

LukeTowers commented 6 years ago

I don't think this is an actual issue because if you have access to the backend, then you can already install whatever plugin you want which would grant you full access to the backend. What would an ideal behaviour for October to do when encountering what you are talking about be?

d4wner commented 6 years ago

Well, at first, thanks for your response, sir.

But if the user config a weak password or use the default password, then the hacker can easily login into the backend, and upload the evil files.

And I've checked the other functions, it seems they don't allow normal user to upload evil php file strictly , isn't it? I think adding rules for something exported from zip files may be better.

For example, we can make a white list for these files , not allowed php files and etc.

I think maybe this will be good, how do you think about it, sir?

LukeTowers commented 6 years ago

I don't think that's a threat model that we need to be considering. If an attacker is able to login to the system under an account that has access to manage/upload themes then no amount of whitelisting zip extraction is going to save you. For example, an attacker in that situation could easily just upload a theme with the following file: myBadTheme/pages/hacks.htm

title=Hacked!
url="/hacked"
==
<?php
    function onRun() {
        die("YOU'VE BEEN HACKED!");
    }
?>
==

Then all they need to do is visit example.com/hacked in their browser and boom. Same thing as a PHP file, but without actually using a PHP file. The onus is on the user to have a secured login, if they have a vulnerable account with this level of access lying around then there's not much we can do to protect them in that case.

d4wner commented 6 years ago

Uhhh, actually I know the part you say , sir. Although there are some secure limits at system login, I still think the vulnerability should be much accounted of for us. For example, if there left an sqlinjection vulnerability at the system backend, we still need to fix it, right?

As regards the solution,I think at this part: myBadTheme/pages/hacks.htm We could avoid to get the content directly and parse it by php engine, or we could filter it by black list, after all it should be just a simple htm file.

Exactly, I admit a not so important vulnerability like strored-type xss here should be ignored.

But if the hacker can control system by the function, it could be a very bad thing at last, I think we should do something to solve it ,even if we don't have a very nice solution for it now, or we could not do it in the recent versions, isn't it?

LukeTowers commented 6 years ago

I think you're missing my point, if someone has access to upload theme files, then it doesn't matter what we do, it's already game over at that point. Any theme page, partial, or layout file can contain PHP code that has full access to the system, irregardless of whether the extension is .php or not.

Are you saying that this issue is not present in Build 419 or earlier?

d4wner commented 6 years ago

Ok, maybe we just have different opinions at this point. And I just stand on black-box pentester's point of view to treat it. Anyway, thx for your response , sir.

LukeTowers commented 6 years ago

I appreciate your willingness and time spent testing October for these things. I'll ask @daftspunk about this, but I don't think I would consider this an issue. By all means, please keep testing October for security vulnerabilities, but please consider reporting them directly to either @daftspunk or myself first. My email is in my profile.

daftspunk commented 6 years ago

@LukeTowers is correct in his assertions. Having the ability to upload a theme grants the ability to execute PHP on the server, just like the "Code" tab on any CMS template. If necessary, this is easily restricted by using a permission structure, or by simply having a read-only file system (which is pretty common for high security sites).

Some very custom themes use PHP files in them so blocking this file type is not a good idea either, even though as we've established it wouldn't change anything.