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

CVE-2018-7172 Patch bypass #124

Closed
ASDWQad opened this issue Mar 23, 2019 · 25 comments
Closed

CVE-2018-7172 Patch bypass #124

ASDWQad opened this issue Mar 23, 2019 · 25 comments
Labels

Comments

@ASDWQad
Copy link

ASDWQad commented Mar 23, 2019

deleteFileThemePluginAction()
$filename = isset($_REQUEST[$request]) ? str_ireplace(['./', '../', '..', '', '/'], null, trim($_REQUEST[$request])) : false;

$file="as/../as.php";
$a=str_ireplace(['./', '../', '..', '', '/'], null, trim($file));
var_dump($a); //string(8) "as/../as.php"

@robiso
Copy link
Collaborator

robiso commented Mar 23, 2019

This was already fixed some time ago with: (realpath).
64efdc4

Can you please provide bit of explanation on why you think this fix is necessary or still think bypassing is possible?
Possibly a PoC?

@ASDWQad
Copy link
Author

ASDWQad commented Mar 23, 2019

Well! Maybe I said is not very detailed.Updated patch can still be bypassed.

list($folder, $request) = $entry;
$filename = isset($_REQUEST[$request]) ? str_ireplace(['./', '../', '..', '~', '~/'], null, trim($_REQUEST[$request])) : false;
if (!$filename || empty($filename)) {
    continue;
}
if ($filename == wCMS::get('config', 'theme')) {
	wCMS::alert('danger', 'Cannot delete currently active theme.');
	wCMS::redirect();
	continue;
}
var_dump("{$folder}/{$filename}");die;

Poc: /?deleteFile=/../index.php&token=343e9fd453a9d9895a2c9f616426fb190233ed164ba99d2a1d3bac5a434f165f

var_dump function info:

string 'D:\xampp\htdocs\wondercms/files//../index.php' (length=45)

@ASDWQad
Copy link
Author

ASDWQad commented Mar 23, 2019

Well! Maybe I said is not very detailed.Updated patch can still be bypassed.
list($folder, $request) = $entry;
$filename = isset($_REQUEST[$request]) ? str_ireplace(['./', '../', '..', '', '/'], null, trim($_REQUEST[$request])) : false;
if (!$filename || empty($filename)) {
continue;
}
if ($filename == wCMS::get('config', 'theme')) {
wCMS::alert('danger', 'Cannot delete currently active theme.');
wCMS::redirect();
continue;
}
var_dump("{$folder}/{$filename}");die;

Poc: /?deleteFile=/../index.php&token=343e9fd453a9d9895a2c9f616426fb190233ed164ba99d2a1d3bac5a434f165f

var_dump function info:

string 'D:\xampp\htdocs\wondercms/files//../index.php' (length=45)

1 similar comment
@ASDWQad
Copy link
Author

ASDWQad commented Mar 23, 2019

Well! Maybe I said is not very detailed.Updated patch can still be bypassed.
list($folder, $request) = $entry;
$filename = isset($_REQUEST[$request]) ? str_ireplace(['./', '../', '..', '', '/'], null, trim($_REQUEST[$request])) : false;
if (!$filename || empty($filename)) {
continue;
}
if ($filename == wCMS::get('config', 'theme')) {
wCMS::alert('danger', 'Cannot delete currently active theme.');
wCMS::redirect();
continue;
}
var_dump("{$folder}/{$filename}");die;

Poc: /?deleteFile=/../index.php&token=343e9fd453a9d9895a2c9f616426fb190233ed164ba99d2a1d3bac5a434f165f

var_dump function info:

string 'D:\xampp\htdocs\wondercms/files//../index.php' (length=45)

@ASDWQad
Copy link
Author

ASDWQad commented Mar 23, 2019

There are two characters that can't be written. The one above the Tab key.
Here is denoted by A
/.A.A/index.php

@NicolasCARPi
Copy link

Hello,

I cannot reproduce the issue on the master branch. Trying this request:

/?deleteFile=/../index.php&token=343e9fd453a9d9895a2c9f616426fb190233ed164ba99d2a1d3bac5a434f165f

(with my token of course), doesn't lead to the index.php file being deleted.

There are two characters that can't be written. The one above the Tab key.

We all have different keyboards, use correct code escaping syntax or give us the ASCII number of this character. For me, above the Tab key I have "$".

@robiso
Copy link
Collaborator

robiso commented Mar 26, 2019

@ashesafe, since this can't be reproduced with the given data, can you please provide the ASCII number of the character you've used?

You can use a simple online service such as https://www.browserling.com/tools/text-to-ascii to get the correct ASCII character so we can try to reproduce this again.

@ASDWQad
Copy link
Author

ASDWQad commented Mar 26, 2019

ascill 126
Poc: /?deleteFile=/.126.126/index.php&token=343e9fd453a9d9895a2c9f616426fb190233ed164ba99d2a1d3bac5a434f165f

@robiso
Copy link
Collaborator

robiso commented Mar 26, 2019

@ashesafe, thank you.

I could now successfully reproduce this with the "tilde" character:
?deleteFile=/.~.~/index2.php&token=792ff832043b345wef345fca9011aaa9e5aa4e042ad7248461

@robiso
Copy link
Collaborator

robiso commented Mar 26, 2019

@ashesafe, mind sharing your first/last name and Twitter link?
You'll be added to the WonderCMS contributors wall of fame https://www.wondercms.com/special-contributors and a thanks note in the changelog https://www.wondercms.com/whatsnew once we push an update.

@ASDWQad
Copy link
Author

ASDWQad commented Mar 26, 2019

Very thankful.But my English is not good.My name is Ashe and No twitter account.

@robiso
Copy link
Collaborator

robiso commented Mar 26, 2019

@ashesafe, do you possibly have a website link? Don't worry about it!

Was the ~ the only character which you could reproduce this? What was the other character you mentioned?

Additionally, can I ask you put the patch you posted as the first comment #124 (comment) in code blocks, so it will display correctly?

@NicolasCARPi
Copy link

Can't reproduce it with nginx, maybe it's an Apache related issue?

@ASDWQad
Copy link
Author

ASDWQad commented Mar 27, 2019

@ashesafe, do you possibly have a website link? Don't worry about it!

Was the ~ the only character which you could reproduce this? What was the other character you mentioned?

Additionally, can I ask you put the patch you posted as the first comment #124 (comment) in code blocks, so it will display correctly?

My english is not good.But I can understand the general meaning, and I am happy to help maintain the system and follow the author's advice.

@ASDWQad ASDWQad closed this as completed Mar 27, 2019
@ASDWQad
Copy link
Author

ASDWQad commented Mar 27, 2019

Can't reproduce it with nginx, maybe it's an Apache related issue?

The problem is that the code layer has nothing to do with the environment

@NicolasCARPi
Copy link

The problem is that the code layer has nothing to do with the environment

I agree that the code must be improved, but it seems this exploit is only valid for Apache webserver.

@NicolasCARPi NicolasCARPi reopened this Mar 27, 2019
@robiso
Copy link
Collaborator

robiso commented Mar 27, 2019

@NicolasCARPi, did you copy/paste my reproduction line above? This was indeed on Apache.
Reference:
?deleteFile=/.~.~/index2.php&token=792ff832043b345wef345fca9011aaa9e5aa4e042ad7248461
(notice my "index2.php", I copied the index and renamed it to "2", so I didn't delete the primary index.php I was using).

@NicolasCARPi
Copy link

@robiso yep, did that, with my token.

2019-03-27-141909_826x35_scrot

@robiso
Copy link
Collaborator

robiso commented Mar 27, 2019

@NicolasCARPi, I'm surprised the following line doesn't fix this already (as far I understand this character should already be "nulled"?)
$filename = isset($_REQUEST[$request]) ? str_ireplace(['./', '../', '..', '~', '~/', '.~.~'], null, trim($_REQUEST[$request])) : false;
also tried with
str_ireplace(['./', '../', '..', '~', '~/', chr(152)]

@NicolasCARPi
Copy link

Anyway, the logic is flawed. It's never a good idea to blacklist things, it's better to whitelist things.

I would use dirname or something to have the root path, and then basename to make sure we only take into account the last part of the provided path.

@robiso
Copy link
Collaborator

robiso commented Mar 27, 2019

@NicolasCARPi, understood. Can we consider this temporarily patched with:
str_ireplace(['./', '../', '..', '~', '~/', '/']
as I couldn't reproduce it with the above added "patch". It does indeed seem to send the slash to null.

@NicolasCARPi
Copy link

Do what works for a patch on the master branch, and we'll see about improving the whole process in the dev branch ;)

@robiso
Copy link
Collaborator

robiso commented Mar 27, 2019

@NicolasCARPi, sounds like a plan. 🍺

@ashesafe, if you change the following line in your WonderCMS installation:
$filename = isset($_REQUEST[$request]) ? str_ireplace(['./', '../', '..', '~', '~/'], null, trim($_REQUEST[$request])) : false;
to
$filename = isset($_REQUEST[$request]) ? str_ireplace(['./', '../', '..', '~', '~/', '/'], null, trim($_REQUEST[$request])) : false;
can you still reproduce this?

If not, I'll push a patch out tomorrow versioned 2.7.0 and a thank you note. I would also like to add you to the special-contributors page. @ashesafe, do you possibly have a website link?
Otherwise I will just publish the note to "thanks to Ashe Safe".

@ASDWQad ASDWQad closed this as completed Mar 28, 2019
@ASDWQad
Copy link
Author

ASDWQad commented Mar 28, 2019

@NicolasCARPi, sounds like a plan. 🍺

@ashesafe, if you change the following line in your WonderCMS installation:
$filename = isset($_REQUEST[$request]) ? str_ireplace(['./', '../', '..', '', '/'], null, trim($_REQUEST[$request])) : false;
to
$filename = isset($_REQUEST[$request]) ? str_ireplace(['./', '../', '..', '', '/', '/'], null, trim($_REQUEST[$request])) : false;
can you still reproduce this?

If not, I'll push a patch out tomorrow versioned 2.7.0 and a thank you note. I would also like to add you to the special-contributors page. @ashesafe, do you possibly have a website link?
Otherwise I will just publish the note to "thanks to Ashe Safe".

Although this is a good idea,But under Window \ can still cross the directory.You can be considered in filtering \ characters

$aa=unlink('.\1111111111.txt');
var_dump($aa);

@ASDWQad
Copy link
Author

ASDWQad commented Mar 28, 2019

@NicolasCARPi, sounds like a plan. 🍺

@ashesafe, if you change the following line in your WonderCMS installation:
$filename = isset($_REQUEST[$request]) ? str_ireplace(['./', '../', '..', '', '/'], null, trim($_REQUEST[$request])) : false;
to
$filename = isset($_REQUEST[$request]) ? str_ireplace(['./', '../', '..', '', '/', '/'], null, trim($_REQUEST[$request])) : false;
can you still reproduce this?

If not, I'll push a patch out tomorrow versioned 2.7.0 and a thank you note. I would also like to add you to the special-contributors page. @ashesafe, do you possibly have a website link?
Otherwise I will just publish the note to "thanks to Ashe Safe".

Web link:https://www.cnblogs.com/ashe666

@robiso robiso added the fixed label Apr 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants