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

Bcrypt password must not contain null character #1311

Closed
Masked-Witch opened this issue Apr 18, 2024 · 9 comments
Closed

Bcrypt password must not contain null character #1311

Masked-Witch opened this issue Apr 18, 2024 · 9 comments
Assignees
Labels

Comments

@Masked-Witch
Copy link

I tried my hand at setting up a new Movim pod following INSTALL.md. Here's the software I'm using

OS: Debian 12
Web Server: Apache2 2.4.59-1
PHP: 8.2+93
PostgreSQL: 15+248

When setup the SQL database with these commands

CREATE USER movim WITH PASSWORD 'removed' CREATEDB;
CREATE DATABASE movimdb TEMPLATE template0 ENCODING 'UTF8';
ALTER DATABASE movimdb OWNER TO movim;
GRANT ALL PRIVILEGES ON DATABASE movimdb TO movim;
GRANT ALL PRIVILEGES ON SCHEMA public TO movim;

I get this error after following the instructions and setting up the database

movim[199869]: movim.ERROR: Bcrypt password must not contain null character [] []

No passwords I created used any special or null characters. Please may I get help on this?

@tyler-hh
Copy link

Ditto.
I did a git pull on movim last weekend.

[2024-04-19T14:29:30.596724+00:00] movim.ERROR: Bcrypt password must not contain null character

Ubuntu 22.04.4 LTS
nginx version: nginx/1.18.0 (Ubuntu)
PHP 8.3.6 (cli) (built: Apr 11 2024 20:23:38) (NTS)

My instance was running fine until I upgraded my php and did a machine reboot.

There are other users within the Movim MUC that are having the issue as well.

@roughnecks
Copy link

I have the same Bcrypt error message at login

Running Debian 12 Stable with nginx, postgres and PHP current versions.

@poVoq
Copy link

poVoq commented Apr 19, 2024

Seems to be the change: https://fossies.org/diffs/php/8.2.17_vs_8.2.18/ext/standard/password.c-diff.html

I am still on 8.2.17 on Fedora 38 and the issue is not happening (yet).

@edhelas edhelas added the bug label Apr 19, 2024
@pic2debug
Copy link

pic2debug commented Apr 22, 2024

I have the same Bcrypt error message at login. Manually downgraded php version does not fix errors.

Movim: v0.22.5
OS: Linux version 6.1.0-20-amd64 (debian-kernel@lists.debian.org) (gcc-12 (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40) SMP PREEMPT_DYNAMIC Debian 6.1.85-1 (2024-04-11)
Nginx: 1.22.1-9
PHP: 8.2.18-1~deb12u1
PostgreSQL: 15+248

toastal added a commit to toastal/nixpkgs that referenced this issue Apr 24, 2024
Note: this does *not* fix the `bcrypt` issue with Nixpkgs’s PHP being
ahead of what Movim’s maintainers are using currently

movim/movim#1311
toastal added a commit to toastal/nixpkgs that referenced this issue Apr 24, 2024
Note: this does *not* fix the `bcrypt` issue with Nixpkgs’s PHP being
ahead of what Movim’s maintainers are using currently

movim/movim#1311
@dos1
Copy link
Contributor

dos1 commented Apr 24, 2024

In app/Session.php:

    public static function hashSession(string $username, string $password, string $host): string
    {
        return $username . "\0" . $password . "\0" . $host;
    }

Frankly, this should have been a 0.24 blocker 👀

@roughnecks
Copy link

Frankly, this should have been a 0.24 blocker

👍

@Masked-Witch
Copy link
Author

Masked-Witch commented Apr 25, 2024

I am not that familiar with php. Is there an issue with reverting 69c2280 ?

@pic2debug
Copy link

pic2debug commented Apr 25, 2024

Please do not try this in a production environment.

Movim upgraded to 0.24 and the same Bcrypt error message is still seen at login.
I edited line 157 of the app/Session.php to make the following change:

    public static function hashSession(string $username, string $password, string $host): string
    {
        // return $username . "\0" . $password . "\0" . $host;
        return $username . "\e" . $password . "\e" . $host;
    }

Finally, the bcrypt error message was not seen again. Login behavior returns to normal. I don't know if it increases the security risk. It works for me.

When we upgrade PHP to 8.2.18, PHP no longer allows us to use "\0" for the session hash.

I don't know why the '|' was replaced. Isn't that a '|' confusing? The user can set the $password , $password might have a "|" , so we don't use '|' ? What is the function of the "|" symbol? Is it just a separator? If the character is just to make the code easier to read, we can use other characters. Why did edhelas choose "\0" instead of other characters in 69c2280 ? Can someone teach me? I am not that familiar with php.

I actually don't know how to fix [#1147 ]. According to the description on the PHP manual, a random salt will be generated by password_hash() for each string hashed. We only need to upgrade the php version to above 8.0. The threat mentioned in #1147 no longer exists, even if we have not change the movim code at all.

@TheBluestBird
Copy link

I've been asked to report my setup here, for I also reproduce this bug.
OS: Archlinux
Web Server: Apache httpd 2.4.59
PHP: 8.3.6
MariaDB: 11.3.2

P.S. Tried @pic2debug solution - worked like a charm

@edhelas edhelas self-assigned this Apr 28, 2024
toastal added a commit to toastal/nixpkgs that referenced this issue Apr 28, 2024
The the issues on Movim’s proprietary issue tracker:
movim/movim#1311
toastal added a commit to toastal/nixpkgs that referenced this issue Apr 28, 2024
See the issues on Movim’s proprietary issue tracker:
movim/movim#1311
toastal added a commit to toastal/nixpkgs that referenced this issue Apr 28, 2024
See the issue on Movim’s proprietary issue tracker:
movim/movim#1311
toastal added a commit to toastal/nixpkgs that referenced this issue Apr 28, 2024
See the issue on Movim’s proprietary issue tracker:
movim/movim#1311
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

8 participants