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
ashesafe opened this Issue Mar 23, 2019 · 25 comments

Comments

Projects
None yet
3 participants
@ashesafe
Copy link

ashesafe 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

This comment has been minimized.

Copy link
Owner

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?

@ashesafe

This comment has been minimized.

Copy link
Author

ashesafe 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)
@ashesafe

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Author

ashesafe 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)
@ashesafe

This comment has been minimized.

Copy link
Author

ashesafe 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

This comment has been minimized.

Copy link
Collaborator

NicolasCARPi commented Mar 24, 2019

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

This comment has been minimized.

Copy link
Owner

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.

@ashesafe

This comment has been minimized.

Copy link
Author

ashesafe commented Mar 26, 2019

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

@robiso

This comment has been minimized.

Copy link
Owner

robiso commented Mar 26, 2019

@ashesafe, thank you.

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

@robiso

This comment has been minimized.

Copy link
Owner

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.

@ashesafe

This comment has been minimized.

Copy link
Author

ashesafe commented Mar 26, 2019

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

@robiso

This comment has been minimized.

Copy link
Owner

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

This comment has been minimized.

Copy link
Collaborator

NicolasCARPi commented Mar 27, 2019

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

@ashesafe

This comment has been minimized.

Copy link
Author

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

@ashesafe ashesafe closed this Mar 27, 2019

@ashesafe

This comment has been minimized.

Copy link
Author

ashesafe 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

This comment has been minimized.

Copy link
Collaborator

NicolasCARPi commented Mar 27, 2019

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

This comment has been minimized.

Copy link
Owner

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

This comment has been minimized.

Copy link
Collaborator

NicolasCARPi commented Mar 27, 2019

@robiso yep, did that, with my token.

2019-03-27-141909_826x35_scrot

@robiso

This comment has been minimized.

Copy link
Owner

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

This comment has been minimized.

Copy link
Collaborator

NicolasCARPi commented Mar 27, 2019

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

This comment has been minimized.

Copy link
Owner

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

This comment has been minimized.

Copy link
Collaborator

NicolasCARPi commented Mar 27, 2019

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

@robiso

This comment has been minimized.

Copy link
Owner

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

@ashesafe ashesafe closed this Mar 28, 2019

@ashesafe

This comment has been minimized.

Copy link
Author

ashesafe 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);
@ashesafe

This comment has been minimized.

Copy link
Author

ashesafe 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
You can’t perform that action at this time.