Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.htaccess files allowed in themes #3257

Closed
d4wner opened this issue Nov 22, 2017 · 8 comments
Closed

.htaccess files allowed in themes #3257

d4wner opened this issue Nov 22, 2017 · 8 comments

Comments

@d4wner
Copy link

d4wner commented Nov 22, 2017

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
Copy link
Contributor

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?

@LukeTowers LukeTowers changed the title Octobercms left an arbitrary upload vulnerability in the latest version. .htaccess files allowed in themes Nov 22, 2017
@d4wner
Copy link
Author

d4wner commented Nov 23, 2017

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
Copy link
Contributor

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
Copy link
Author

d4wner commented Nov 23, 2017

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
Copy link
Contributor

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
Copy link
Author

d4wner commented Nov 24, 2017

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
Copy link
Contributor

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
Copy link
Member

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants