Wrong login/password since v0.5.2 #335

dper · 2015-09-02T13:24:56Z

If I upgrade to v0.5.2, whenever I try to log in I get the popup window showing the error message, The page at [my domain] says: Wrong login/password. This started happening recently. If I check out v0.5.1 or earlier, the login process works normally as it has in the past.

My web server is Dreamhost.com, a commercial web host. (I can to some degree provide more details of the setup on Dreamhost, but since it's a third party host, there are things outside of my control even if we know what's going wrong. If things seem to be heading in that direction, we might be better off not spending much time on this.)
PHP 5.4.42.
Apache 2.2.22-21.

I take a browser and clear the memory. I load my instance of Shaarli. Let's suppose it's "mydomain.org". Before trying to log in, I examine the cookies. There are two, shaarli and shaarli_staySignedIn.

Here is access.log when I try to log in and fail.

xxx.xxx.xxx.xxx - - [02/Sep/2015:06:19:11 -0700] "GET /bookmarks/?do=login HTTP/1.1" 200 5718 "-" "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/44.0.2403.157 Safari/537.36" 
xxx.xxx.xxx.xxx - - [02/Sep/2015:06:19:19 -0700] "POST /bookmarks/?do=login HTTP/1.1" 200 849 "https://mydomain.org/bookmarks/?do=login" "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/44.0.2403.157 Safari/537.36" 
xxx.xxx.xxx.xxx - - [02/Sep/2015:06:19:29 -0700] "GET /bookmarks/?do=login HTTP/1.1" 200 3252 "https://mydomain.org/bookmarks/?do=login" "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/44.0.2403.157 Safari/537.36"

Nothing posts to error.log.

I have tried using both Iceweasel (Firefox) and Chromium. I have also tried enabling 3rd party cookies. The results are the same. Let me know if there's any other information I can provide.

The text was updated successfully, but these errors were encountered:

ArthurHoaro · 2015-09-02T13:29:36Z

Can you make sure that you're shaarli cookie is attached to the hostname you're visiting in your browser?

EDIT: also, what's its value?

dper · 2015-09-02T14:15:05Z

Name: shaarli
Content: LEVQF9tKKPN6oBX4wShqh2
Domain: .mydomain.org
Path: /bookmarks/
Send for: Any kind of connection
...

The content of the cookie changes each time I open the browser. (I also have the browser set to clear cookies on exit.) In the web browser, I can see no substantive differences between the cookies when switching between v0.5.1 and v0.5.2.

ArthurHoaro · 2015-09-02T14:25:48Z

Holy crap! In application/Utils.php, line 159, add i at the end of the regexp

if (!preg_match('/^[a-z0-9]{2,32}$/i', $sessionId)) {

And let me know if it's working.

dper · 2015-09-02T14:33:10Z

@ArthurHoaro If I add the i as you wrote, then it works. (Also, that was some quick work. Thanks!)

Fixes shaarli#335 - Wrong login/password since v0.5.2 Regression introduced in 06b6660

virtualtam · 2015-09-02T15:50:00Z

Thanks @dper for the feedback & @ArthurHoaro for the quick fix ;-)

nodiscc · 2015-09-02T16:00:21Z

👍 well done (? I don't understand what the problem was)

dper · 2015-09-03T01:52:39Z

@nodiscc I believe the situation was this... The trailing /i (instead of just /) makes the regular expression case insensitive. The value of the cookie in my browser contained numbers and upper case letters, and as a security measure Shaarli was expecting it to be numbers and only lower case letters, so it was rejecting login attempts. Since upper case letters are valid, the fix is to accept them, too.

ArthurHoaro · 2015-09-03T09:08:42Z

Exactly.

Cy4n1d3 · 2015-09-03T19:39:53Z

Sadly the latest release didn't fix it for me.
I've bisected the commits and identified 06b6660 to be the cause (as @ArthurHoaro already pointed out) - 0.5.1 is the latest working release for me. The latest hotfix however still does not allow me to log in again.

L388 @ index.php (tokenOk($_POST['token'])) is where the login fails.
I'll dig into the code and see if I can find what exactly goes wrong.

€dit:
The culprit is still L159 @ Utils.php. The regex fails.

Cy4n1d3 · 2015-09-03T20:10:56Z

I guess I've figured out why the check is failing for me.
My PHP is configured to use SHA512 as hash function for the $_SESSION-id, instead of the default md5 / sha1.
The corresponding setting is
session.hash_function = sha512
inside php.ini.

I've temporarily commented that setting out and was able to log in again.

What does that give us?
First and foremost: that regex is not really error proof :)

Have a look at the following PHP setting: https://secure.php.net/manual/en/session.configuration.php#ini.session.hash-bits-per-character

By settings session.hash_function = sha512 and session.hash_bits_per_character = 6 the resulting session id would look something along the lines of iJXJWB-H1w-QhXGZlJsBPQnaeAGaG,SmZFcYxYsxtjXdlslXQn8UbeyHB0ZkGZeiN5ZAsUsRc4W38YcICkTe11.
That string has a length of 88 (fails {2,32}) and even contains special characters (fails [a-z0-9]).
Depending on the server configuration the string can have a length of up to 128 chars it seems.

A regex which is working for my configuration would be /^[-,a-zA-Z0-9]{1,128}$/.

You might also construct the regex on the fly, depending on php.ini settings. strlen(session_id()) should also retrieve the length of the servers' session id in most cases.

virtualtam · 2015-09-03T20:19:56Z

Hello @Cy4n1d3 and thanks for reporting and root-causing the regression :)

Possible solutions:

implement a UUID generator
use sha1(uniqid()) for the shaarli cookie

Cy4n1d3 · 2015-09-03T20:25:49Z

Thank you guys, for keeping this project up and running ;)
An error report is the least I can do!

Improves shaarli#306 Relates to shaarli#335 & shaarli#336 Issue: - the format of the value returned by `uniqid()` depends on PHP settings - the regex checking the session ID does not cover all cases Fix: - apply a hash function to the session ID (SHA1) See: - http://php.net/manual/en/session.configuration.php#ini.session.hash-function Signed-off-by: VirtualTam <virtualtam@flibidi.net>

virtualtam · 2015-09-03T21:23:49Z

PR #338 should fix this issue

(by the way, sha1(uniqid()) is already used in several places in index.php)

Improves shaarli#306 Relates to shaarli#335 & shaarli#336 Duplicated by shaarli#339 Issues: - PHP regenerates the session ID if it is not compliant - the regex checking the session ID does not cover all cases - different algorithms: md5, sha1, sha256, etc. - bit representations: 4, 5, 6 Fix: - regex: support all possible characters - '[a-zA-Z,-]{2,128}' - tests: add coverage for all algorithms & bit representations TODO: - remove `uniqid()` usage See: - http://php.net/manual/en/session.configuration.php#ini.session.hash-function - https://secure.php.net/manual/en/session.configuration.php#ini.session.hash-bits-per-character - http://php.net/manual/en/function.session-id.php - http://php.net/manual/en/function.session-regenerate-id.php - http://php.net/manual/en/function.hash-algos.php Signed-off-by: VirtualTam <virtualtam@flibidi.net>

Improves shaarli#306 Relates to shaarli#335 & shaarli#336 Duplicated by shaarli#339 Issues: - PHP regenerates the session ID if it is not compliant - the regex checking the session ID does not cover all cases - different algorithms: md5, sha1, sha256, etc. - bit representations: 4, 5, 6 Fix: - `index.php`: - remove `uniqid()` usage - call `session_regenerate_id()` if an invalid cookie is detected - regex: support all possible characters - '[a-zA-Z,-]{2,128}' - tests: add coverage for all algorithms & bit representations See: - http://php.net/manual/en/session.configuration.php#ini.session.hash-function - https://secure.php.net/manual/en/session.configuration.php#ini.session.hash-bits-per-character - http://php.net/manual/en/function.session-id.php - http://php.net/manual/en/function.session-regenerate-id.php - http://php.net/manual/en/function.hash-algos.php Signed-off-by: VirtualTam <virtualtam@flibidi.net>

ArthurHoaro added bug it's broken! unconfirmed labels Sep 2, 2015

ArthurHoaro removed the unconfirmed label Sep 2, 2015

ArthurHoaro added a commit to ArthurHoaro/Shaarli that referenced this issue Sep 2, 2015

Allow uppercase letters in PHP sessionid format

4d30975

Fixes shaarli#335 - Wrong login/password since v0.5.2 Regression introduced in 06b6660

ArthurHoaro mentioned this issue Sep 2, 2015

Allow uppercase letters in PHP sessionid format #336

Merged

ArthurHoaro closed this as completed in #336 Sep 2, 2015

virtualtam added this to the 0.5.3 milestone Sep 2, 2015

virtualtam added the security label Sep 2, 2015

virtualtam mentioned this issue Sep 3, 2015

Session ID: extend the regex to match possible hash representations #338

Merged

Cy4n1d3 mentioned this issue Sep 4, 2015

Session: use session_regenerate_id() to obtain a fresh session id and… #339

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wrong login/password since v0.5.2 #335

Wrong login/password since v0.5.2 #335

dper commented Sep 2, 2015

ArthurHoaro commented Sep 2, 2015

dper commented Sep 2, 2015

ArthurHoaro commented Sep 2, 2015

dper commented Sep 2, 2015

virtualtam commented Sep 2, 2015

nodiscc commented Sep 2, 2015

dper commented Sep 3, 2015

ArthurHoaro commented Sep 3, 2015

Cy4n1d3 commented Sep 3, 2015

Cy4n1d3 commented Sep 3, 2015

virtualtam commented Sep 3, 2015

Cy4n1d3 commented Sep 3, 2015

virtualtam commented Sep 3, 2015

Wrong login/password since v0.5.2 #335

Wrong login/password since v0.5.2 #335

Comments

dper commented Sep 2, 2015

ArthurHoaro commented Sep 2, 2015

dper commented Sep 2, 2015

ArthurHoaro commented Sep 2, 2015

dper commented Sep 2, 2015

virtualtam commented Sep 2, 2015

nodiscc commented Sep 2, 2015

dper commented Sep 3, 2015

ArthurHoaro commented Sep 3, 2015

Cy4n1d3 commented Sep 3, 2015

Cy4n1d3 commented Sep 3, 2015

virtualtam commented Sep 3, 2015

Cy4n1d3 commented Sep 3, 2015

virtualtam commented Sep 3, 2015