Dotclear

Ticket #2238 (closed defect: wontfix)

Opened 7 years ago

Last modified 7 years ago

Security Problem In Language Package Uploading

Reported by: bajinsheng Owned by: team
Priority: highest Milestone:
Component: security Version: 2.11.2
Severity: major Keywords: File Upload Attack
Cc:

Description

I find a security problem on uploading language package model. The uploaded zip file can contain webshell backdoor without filtration.

Attack method: 1.I first login in using super administrator account. In the page: “/admin/langs.php”, I uploaded the aa-aa.zip, which compress the folder(named "aa-aa") including virus.php(webshell file) and main.po(empty file). 2.After the zip file uploaded success, get the url: “/locales/aa-aa/virus.php?cmd=phpinfo();”. We can get the webshell.

The key code location: 1.On the page:/admin/langs.php, line 119: $ret_code = dc_lang_install($dest); When administrator upload the zip file, the function check the password and then extract the zip file by dc_lang_install(). 2.On the page:/admin/langs.php, line 295:$zip->unzipAll($target); In the dc_lang_install() function. After check the folder name and main.po file, the program call the unzipAll() function. 3.On the page:/inc/libs/clearbricks/zip/class.unzip.php, line 87:$this->unzip($k,$target.'/'.$k); In the unzipAll() function, extract every file by calling unzip() function. 4.On the page:/inc/libs/clearbricks/zip/class.unzip.php, line 100:if ($this->isFileExcluded($file_name)) In th unzip() function, it check every file and determine whether the file executable file by calling isFileExcluded() function. In the isFileExcluded() function(line 215), it use exclude_pattern variable to match the string, but in the line 36(protected $exclude_pattern = ;), so the isFileExcluded() function do not work.

Attached here to the uploaded zip file.

Jinsheng Ba bajinsheng@…

Attachments

aa-aa.zip Download (3.1 KB) - added by bajinsheng 7 years ago.
attach file

Change History

Changed 7 years ago by bajinsheng

attach file

comment:1 follow-up: ↓ 2 Changed 7 years ago by franck

Hi,

Thanks for your report.

Could you explain me how you gain access to the super-admin account needed to upload such archive?

comment:2 in reply to: ↑ 1 Changed 7 years ago by bajinsheng

Replying to franck:

Hi,

Thanks for your report.

Could you explain me how you gain access to the super-admin account needed to upload such archive?

Thanks for your reply. Firstly, about how i get the super-admin account. Indeed, it looks impossibility to get the account. But it just need lots of time. In the page:/admin/auth.php, there is a model called "forget your password", which through a key to reset password. In the page:/dotclear/inc/core/class.dc.auth.php line 574:$key = md5(uniqid());. The key generated by uniqid() function, so the key may non-uniqueness and regular. With the help of no valid time limit in the key, the brute force may the right way to get the key without the email content. The next step, to reset the password of admin. On the page:/inc/libs/clearbricks/common/lib.crypt.php line 78:$pos2 = array_rand(array(4,5,6,7));. the variable pos2 also return the number in(0,1,2,3), the same as variable pos1, so the random password range narrowed. Secondly, even though we can not get the super-admin account, the uploading operation let the administrator get many privileges which not allowed by administrator. Surely it doesn't mean that it's a privilege escalation attack?

comment:3 follow-up: ↓ 5 Changed 7 years ago by franck

  • Severity changed from critical to major

« The key generated by uniqid() function, so the key may non-uniqueness and regular. With the help of no valid time limit in the key, the brute force may the right way to get the key without the email content. »

Ok for the uniqid() — I will enforce a little bit this —, but anyway, bruteforcing a md5 hash will take a very very long time (up to 2128 combinations to test(1)), who will take all this time to bruteforcing the installation?

We also have in mind to enforce password management, using the native PHP 5.5 password hashing function (see #2182, in french).

(1) Assuming 27 gigahashes/second → 2128 / 27*109 will last about 4*1018 centuries

Last edited 7 years ago by franck (previous) (diff)

comment:4 Changed 7 years ago by franck <carnet.franck.paul@…>

(In [5e23e26bfd45]) Enforce uniqness of the recovery key, addresses #2238

comment:5 in reply to: ↑ 3 Changed 7 years ago by bajinsheng

Replying to franck:

« The key generated by uniqid() function, so the key may non-uniqueness and regular. With the help of no valid time limit in the key, the brute force may the right way to get the key without the email content. »

Ok for the uniqid() — I will enforce a little bit this —, but anyway, bruteforcing a md5 hash will take a very very long time (up to 2128 combinations to test(1)), who will take all this time to bruteforcing the installation?

We also have in mind to enforce password management, using the native PHP 5.5 password hashing function (see #2182, in french).

(1) Assuming 27 gigahashes/second → 2128 / 27*109 will last about 4*1018 centuries

Maybe my method is not rigorous, but i want to express is that the return value of uniqid() is regular, not bruteforcing a md5 hash. First get local uniqid() function return value, then get uniqid() return value in web server. their values will not differ too much, and bruteforcing it.

All the above is to answer how to get admin account, and i don't commit it because i also think it is no big problem. But i believe the language package uploading problem is dangerous. So whether we deviate from the topic? I believe some other safety protection measures in super-admin dashboard can prove you also agree the opinion that the super-admin also not allowed to do everything in web server.

comment:6 follow-up: ↓ 7 Changed 7 years ago by franck

Ok it will be possible to replace uniqid() by openssl_random_pseudo_bytes(128) for example and then it will be far more difficult to predict the source of the md5 function.

Anyway, about super-admin: yes it should be protected for the more evident potential problems (essentially as the common and only user of a dotclear mono-blog installation is the super-admin, and often not very aware about potential risks), if possible, but he is super-admin (as the root is on a linux system) and then it is not more the responsability of dotclear to disallow him to do what he wants on the system.

The super-admin had/has the rights to install dotclear (via FTP/ssh/…) and so he already can do what he wants, whether or not dotclear is installed.

About this very topic: in order to upload a language archive file, your must be a super-admin, so…

comment:7 in reply to: ↑ 6 Changed 7 years ago by bajinsheng

Replying to franck: It may be sure that the super-admin couldn't deliberately upload this malicious language pack. But if the super-admin use the language package made by a hacker while he don't know it? And also, if the password of admin is leaked, that is to say the leaked account is the same as the root account of the web server? you say the super-admin should have the responsibility to disallow him some operation, but this is a technology problem, not human problem.

I report the problem just want your software get better and security. As you say it should be protected for the more evident potential problems, why not repair it?

comment:8 follow-up: ↓ 9 Changed 7 years ago by franck

Having its password leaked, having an super-admin uploading a package which is not verified before, … are human problems imho.

Indeed we would like to secure as far as possible our software, but it may consume a lot of resources in order to prevents such potential problems. How controlling the content of a language pack that may include a lot of files and folders ? How to control the very content of each of these files ? Could you tell me how you control that a file is not a legitimate script, and so on…

We will enforce the security but we have to made compromise between amount of code and cpu used and the reasonable security provided by the software.

By the way, any advise, any suggestion of code modification are welcomed, at any time!

comment:9 in reply to: ↑ 8 Changed 7 years ago by bajinsheng

Replying to franck: Ok, could you explain why you thought that is a security problem? http://dev.dotclear.org/2.0/ticket/2214

comment:10 Changed 7 years ago by franck

  • Status changed from new to closed
  • Resolution set to wontfix
  • Milestone A definir deleted
Note: See TracTickets for help on using tickets.

Sites map