View Issue Details

IDProjectCategoryView StatusLast Update
0012957mantisbtldappublic2023-02-27 05:22
Reporterscmme Assigned Todregad  
PriorityhighSeveritytweakReproducibilityhave not tried
Status assignedResolutionopen 
Product Version1.2.5 
Summary0012957: Password stored md5-unsalted in database when LDAP authentication is enabled
Description

When LDAP authentication is enabled the password field in the database will automatically update upon login. This is helpful for systems where the authentication system may be migrated from LDAP to local, but is unnecessary in environments where LDAP is a strict requirement. It exposes additional information in an insecure manner (md5 + no salt).

This behavior should be at least configurable if not stored in a more secure manner.

Steps To Reproduce
  1. Configure LDAP Authentication / Create a user with valid LDAP credentials
  2. update $c_user_table set password = '' where username = 'user'; commit;
  3. Login as user with valid credentials
  4. select password from $c_user_table where username = 'user'
  5. Take note of value 'password'
Additional Information

Minor patch would be to:
Modify function ldap_authenticate_by_username in core/ldap_api.php to wrap the user_set_field password call to require a configuration item, eg:
if ( ON == config_get( 'ldap_db_pwd_autoupdate' ) ) {
user_set_field( $t_user_id, 'password', md5( $p_password ) );
}

Document new variable and set it in configuration.

Tagspatch

Relationships

related to 0015721 closedgrangeway Functionality to consider porting to master-2.0.x 
related to 0022839 assigneddregad Deprecate MD5 login method and replace with BCRYPT hash 
has duplicate 0019393 closedatrol Config-Parameter for LDAP password saving 
has duplicate 0026626 closedatrol Add config option to not cache (insecure MD5) password hashes in the database 
related to 0022156 closedatrol Password are stored in PLAIN TEXT 
related to 0025771 closeddregad LDAP not update password 
related to 0029861 closeddregad LDAPS - no new users possible & password in cleartext 

Activities

scmme

scmme

2011-05-03 21:32

reporter   ~0028743

This issue also occurs when a user is created.

grangeway

grangeway

2012-02-05 07:38

reporter   ~0031127

The version of the ldap api in our next branch [which supports multiple servers] doesn't store the password locally. Storing an LDAP password locally is really a security risk IMO.

atrol

atrol

2012-03-14 18:20

developer   ~0031466

Reopened, there is no "Fixed in Version" and we will have no "Roadmap" and "Changelog". There is no patch / changeset attached which will confuse any user who has a look at this issue.

grangeway

grangeway

2012-10-20 20:00

reporter   ~0033292

this is fixed in the mantis-2.x branch

grangeway

grangeway

2013-04-05 17:56

reporter   ~0036219

Marking as 'acknowledged' not resolved/closed to track that change gets ported to master-2.0.x branch

andy778

andy778

2015-09-09 06:25

reporter   ~0051423

Noticed this bug the hard way and bug is still existing in 1.2.19, is this planned to be fixed?

Think "select md5('XX');" is quite simple today to crack with hashcat.net and some decent graphic card

dregad

dregad

2015-09-09 18:56

developer   ~0051430

I agree this is an issue, and it will get fixed eventually, but TBH it's not very high on my radar at the moment.

If you're able and willing to contribute a patch, it would be more than welcome. The best would be a pull request on Github, or alternatively a unified diff.

atrol

atrol

2015-10-16 01:30

developer   ~0051638

PR at https://github.com/mantisbt/mantisbt/pull/659

atrol

atrol

2016-01-29 10:01

developer   ~0052437

One more PR https://github.com/mantisbt/mantisbt/pull/713

atrol

atrol

2017-01-10 11:43

developer   ~0055027

Latest PR for it https://github.com/mantisbt/mantisbt/pull/718

@dregad @vboctor 2.1 could be a good moment to fix it.

tfnab

tfnab

2018-11-12 04:38

reporter   ~0060946

Folks: this is a security issue. I seriously don't understand why it can't be fixed. It's not like there aren't any suitable patches, pull requests (the latest being https://github.com/mantisbt/mantisbt/pull/718) and the like. What is required to get the proper attention for this?

rogueresearch

rogueresearch

2019-03-18 16:40

reporter   ~0061701

Seems related to: https://mantisbt.org/bugs/view.php?id=22839

rogueresearch

rogueresearch

2020-01-25 16:26

reporter   ~0063518

@atrol @dregad @vboctor what is needed to finally fix either this (0012957) or 0022839?

Do any of you accept money to work on such things? How many man-hours would it take, and what is your hourly rate?

yosh

yosh

2023-02-22 08:53

reporter   ~0067413

I'm using the currently suggested patch on GitHub, I'd be thrilled if an updated version of the code could be merged as I have to maintain a local patch.

Could I assist with this somehow?

rogueresearch

rogueresearch

2023-02-22 11:10

reporter   ~0067416

@yosh As is always the case with open source projects, everyone lacks time, money, code reviewers, and testing bandwidth. So you've been using the patch and it works? The patch still applies cleanly on master? If so, I could apply it to my staging environment and test also...

yosh

yosh

2023-02-22 13:22

reporter   ~0067417

I don't think the patch applies cleanly anymore, I believe I had to shuffle some lines/functions around last time.
I did replace crypto_generate_strong_random_string() with crypto_generate_uri_safe_nonce() (commented in https://github.com/mantisbt/mantisbt/pull/718)

I've been using it without complaints for a long time. I'll try to open a PR based on it when I do my next upgrade.