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

tok bypass and SSRF vulnerability in Dokuwiki #1883

Closed
niubl opened this Issue Feb 28, 2017 · 9 comments

Comments

Projects
None yet
3 participants
@niubl

niubl commented Feb 28, 2017

the tok of fetch.php can be guess in php version above 5.5:
http://simple.com/dokuwiki/lib/exe/fetch.php?media=http://192.168.141.128:80/test.php?test.jpg&tok=0f35df

and attacker can use this make a SSRF attack,vulnerability detail:
http://paper.seebug.org/230/

email:
448354223@qq.com

@splitbrain

This comment has been minimized.

Show comment
Hide comment
@splitbrain

splitbrain Feb 28, 2017

Owner

Could you please submit details in English?

Owner

splitbrain commented Feb 28, 2017

Could you please submit details in English?

@splitbrain

This comment has been minimized.

Show comment
Hide comment
@splitbrain

splitbrain Feb 28, 2017

Owner

I'm trying to figure out what's going on via Google Translate, but 'm not sure I'm getting the details, right.

From what I gather, you're saying you can influence what media_get_token() and getSecurityToken() return by passing an invalid session ID exploiting a bug in PHP.

You state that auth_cookiesalt() is using the sessionID to generate the salt, but that isn't true. It uses random_bytes() and only appends the session ID to that if $addsession is set to true. It's false for both functions above.

Again, please provide an english description and maybe some example code exploiting the problem (feel free to send sensitive data directly to andi@splitbrain.org)

Owner

splitbrain commented Feb 28, 2017

I'm trying to figure out what's going on via Google Translate, but 'm not sure I'm getting the details, right.

From what I gather, you're saying you can influence what media_get_token() and getSecurityToken() return by passing an invalid session ID exploiting a bug in PHP.

You state that auth_cookiesalt() is using the sessionID to generate the salt, but that isn't true. It uses random_bytes() and only appends the session ID to that if $addsession is set to true. It's false for both functions above.

Again, please provide an english description and maybe some example code exploiting the problem (feel free to send sensitive data directly to andi@splitbrain.org)

@splitbrain splitbrain added the Security label Feb 28, 2017

@phy25

This comment has been minimized.

Show comment
Hide comment
@phy25

phy25 Feb 28, 2017

Collaborator

Sad to see this report. Here is some translations to the blog.

DokuWiki fetch.php allows downloading external files with url's param media. There is a signature check with param tok.

checkFileStatus() > media_get_token() > auth_cookiesalt()

The value returned by auth_cookiesalt() is joined by the salt in DokuWiki_htcookiesalt2 and value from session_id(). The practice generating salt by using session_id() is not safe. There is a bug in session_id() in the latest PHP* (older versions are not affected), which may set any value to session_id() by the client cookie (For detail please see the original Chinese blog).

Thus you can guess the value of _htcookiesalt2 easily. For example, use CSRF token generated by getSecurityToken(), which generates token with a similar approach.

Sample code getting token

The bug in PHP has been reported. The bug is not confirmed yet but dev from PHP said it is not good practice to generate salt using session_id.

Collaborator

phy25 commented Feb 28, 2017

Sad to see this report. Here is some translations to the blog.

DokuWiki fetch.php allows downloading external files with url's param media. There is a signature check with param tok.

checkFileStatus() > media_get_token() > auth_cookiesalt()

The value returned by auth_cookiesalt() is joined by the salt in DokuWiki_htcookiesalt2 and value from session_id(). The practice generating salt by using session_id() is not safe. There is a bug in session_id() in the latest PHP* (older versions are not affected), which may set any value to session_id() by the client cookie (For detail please see the original Chinese blog).

Thus you can guess the value of _htcookiesalt2 easily. For example, use CSRF token generated by getSecurityToken(), which generates token with a similar approach.

Sample code getting token

The bug in PHP has been reported. The bug is not confirmed yet but dev from PHP said it is not good practice to generate salt using session_id.

@phy25

This comment has been minimized.

Show comment
Hide comment
@phy25

phy25 Feb 28, 2017

Collaborator

@splitbrain This seems like a public blog post published yesterday. And code samples have been released in the blog post (all the images, which contain no Chinese).

Collaborator

phy25 commented Feb 28, 2017

@splitbrain This seems like a public blog post published yesterday. And code samples have been released in the blog post (all the images, which contain no Chinese).

@splitbrain

This comment has been minimized.

Show comment
Hide comment
@splitbrain

splitbrain Feb 28, 2017

Owner

@phy25 thanks for helping out with translating. But I'm still missing crucial info here. You say:

The value returned by auth_cookiesalt() is joined by the salt in DokuWiki_htcookiesalt2 and value from session_id().

This is only true when passing true in the $addsession parameter. All the functions mentioned in the post, don't do that. And even if it were so, I'm still not seeing how that would allow you to guess the value from _htcookiesalt2. The blog post seems not to explain that part.

Owner

splitbrain commented Feb 28, 2017

@phy25 thanks for helping out with translating. But I'm still missing crucial info here. You say:

The value returned by auth_cookiesalt() is joined by the salt in DokuWiki_htcookiesalt2 and value from session_id().

This is only true when passing true in the $addsession parameter. All the functions mentioned in the post, don't do that. And even if it were so, I'm still not seeing how that would allow you to guess the value from _htcookiesalt2. The blog post seems not to explain that part.

@phy25

This comment has been minimized.

Show comment
Hide comment
@phy25

phy25 Feb 28, 2017

Collaborator

OK I just invesigated further. I just missed an important part in the blog.

getSecurityToken() => PassHash::hmac('md5', session_id().$INPUT->server->str('REMOTE_USER'), auth_cookiesalt());
media_get_token() => substr(PassHash::hmac('md5', $token, auth_cookiesalt()),0,6)

Now auth_cookiesalt() can be treated as a constant value. In most cases $INPUT->server->str('REMOTE_USER') equals to "". If you make session_id() equal to the fetch URL $token, you can get exactly the same token of media_get_token() by getSecurityToken(). Thus the token check can be bypassed.

Collaborator

phy25 commented Feb 28, 2017

OK I just invesigated further. I just missed an important part in the blog.

getSecurityToken() => PassHash::hmac('md5', session_id().$INPUT->server->str('REMOTE_USER'), auth_cookiesalt());
media_get_token() => substr(PassHash::hmac('md5', $token, auth_cookiesalt()),0,6)

Now auth_cookiesalt() can be treated as a constant value. In most cases $INPUT->server->str('REMOTE_USER') equals to "". If you make session_id() equal to the fetch URL $token, you can get exactly the same token of media_get_token() by getSecurityToken(). Thus the token check can be bypassed.

@splitbrain

This comment has been minimized.

Show comment
Hide comment
@splitbrain

splitbrain Feb 28, 2017

Owner

Ohhhh.... now I get it. I saw the weird HTTP url session value in the screenshot, but I didn't make the connection.

Okay. So the actual _htcookiesalt2 is not exposed at all. This is simply using the CSRF mechanism to calculate the token for the media stuff. Clever!

Now I can think about a fix. Thanks for your help.

Owner

splitbrain commented Feb 28, 2017

Ohhhh.... now I get it. I saw the weird HTTP url session value in the screenshot, but I didn't make the connection.

Okay. So the actual _htcookiesalt2 is not exposed at all. This is simply using the CSRF mechanism to calculate the token for the media stuff. Clever!

Now I can think about a fix. Thanks for your help.

@niubl

This comment has been minimized.

Show comment
Hide comment
@niubl

niubl Feb 28, 2017

I am sorry I just go out for dinner and don't see the replay. The vulnerability is not critical becase a SSRF need to set the fetchsize not zero first. By default the fetchsize is zero.

niubl commented Feb 28, 2017

I am sorry I just go out for dinner and don't see the replay. The vulnerability is not critical becase a SSRF need to set the fetchsize not zero first. By default the fetchsize is zero.

splitbrain added a commit that referenced this issue Feb 28, 2017

splitbrain added a commit that referenced this issue Feb 28, 2017

do not use invalid session IDs #1883
When an invalid session ID is passed to PHP a warning is thrown, but the
session is still initialized with this invalid ID (throwing additional
warnings on save).

This makes sure such invalid IDs are removed from the cookie array
before initializing the session.

PHP bug references:

https://bugs.php.net/bug.php?id=68063
https://bugs.php.net/bug.php?id=73860

@niubl niubl closed this Mar 1, 2017

splitbrain added a commit that referenced this issue Mar 1, 2017

splitbrain added a commit that referenced this issue Mar 1, 2017

do not use invalid session IDs #1883
When an invalid session ID is passed to PHP a warning is thrown, but the
session is still initialized with this invalid ID (throwing additional
warnings on save).

This makes sure such invalid IDs are removed from the cookie array
before initializing the session.

PHP bug references:

https://bugs.php.net/bug.php?id=68063
https://bugs.php.net/bug.php?id=73860
@niubl

This comment has been minimized.

Show comment
Hide comment
@niubl

niubl Mar 2, 2017

@splitbrain Will you apply CVE for this vulnerability?

niubl commented Mar 2, 2017

@splitbrain Will you apply CVE for this vulnerability?

bug added a commit that referenced this issue Mar 7, 2017

bug added a commit that referenced this issue Mar 7, 2017

do not use invalid session IDs #1883
When an invalid session ID is passed to PHP a warning is thrown, but the
session is still initialized with this invalid ID (throwing additional
warnings on save).

This makes sure such invalid IDs are removed from the cookie array
before initializing the session.

PHP bug references:

https://bugs.php.net/bug.php?id=68063
https://bugs.php.net/bug.php?id=73860
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment