search

salesagility / SuiteCRM-Core

SuiteCRM - Open source CRM for the world
https://www.suitecrm.com
GNU Affero General Public License v3.0
174 stars 120 forks source link

Invalid passwords can be set #198

Open pow-nico opened 1 year ago

pow-nico commented 1 year ago

Issue

As an administrator I can set a password with special characters. Some of these special characters lead to an invalid password. In consequence a user cannot log in to SuiteCRM. The following error message is shown: "Login credentials incorrect, please try again."

Expected Behavior

It shouldn't be possible to use special characters which lead to an invalid password.

Actual Behavior

It is possible to use any printable special character to create passwords.

Possible Fix

Steps to Reproduce

  1. create a new user
  2. set a password with any of these characters: "`'<>
  3. try to login with these credentials

Context

As this is a bug affecting passwords and user logins I would suggest a HIGH priority

Your Environment

pgorod commented 1 year ago

It is not insecure to allow special characters in passwords. If they're not getting output as HTML (I don't suppose we'll ever print a password on screen), then no HTML clean-ups are necessary. The same goes for JS clean-ups if this isn't used to produce JS code.

The only needed escaping is SQL escaping when it goes into the database - but escaping doesn't mean the characters can't be saved there, this just has to be done properly.

This is part of the general problem of the way clean-ups are done in SuiteCRM which is quite wrong, I've mentioned this on many occasions.

pow-nico commented 1 year ago

I think you are referring "prohibit the usage of some special characters at the password input field (less secure)".

What I meant was:

If you reduce the set of possible characters for a password and keep the same length then you reduce the possible total amount of passwords. That reduces the time for a successful brute force attack. That's why I said "less secure".

Or did you mean anything else?

pgorod commented 1 year ago

Ok, maybe I didn't read you right. As long as there are enough characters in the password, and it's something that can't be constructed from variations upon dictionary entries, then it gets pretty irrelevant which characters are there, in terms of brute-force attacks.

So I don't see this as a security issue, I see it as a bug which creates problems for users and sysadmins.

pow-nico commented 1 year ago

I think we misunderstand each other:

I'm not talking about a theoretical security flaw of SuiteCRM. I'm actually talking about the bug that you can set an invalid password and because of that an user isn't able to login anymore. I described that in detail in the first post and I don't know why you are sticking to that topic "password complexity".

So please read again the first paragraph or reproduce the bug with these simple 3 steps I described in the first post too.

Do you have any questions to that topic? Or what should I describe in more detail?

pow-nico commented 1 year ago

OFFTOPIC "password complexity"

As long as there are enough characters in the password, and it's something that can't be constructed from variations upon dictionary entries, then it gets pretty irrelevant which characters are there, in terms of brute-force attacks

If I only have the characters "a" and "b" for a password with a length of 2 I have 2^2 possible passwords and I have to systematically check a maximum of 4 passwords until I find a valid one.

If I can use the characters {a,b,c,d} for a 2-characters long password then I get 4^2 possible passwords and I have to guess a maximum of 16 passwords.

In the end: it is not irrelevant which characters are there.

And yes a longer password is better then a more complex one. Source

pgorod commented 1 year ago

I just ended my post with this:

So I don't see this as a security issue, I see it as a bug which creates problems for users and sysadmins.

After that, you insist that it's a bug (that's what I am saying also), and ask why I am going on about password complexity. It was because you mentioned it ("less secure"). And then you added to the password complexity topic. Ok.

Off-topic: I am not claiming that less character possibilities is strictly absolutely irrelevant, I am just claiming that the difference between allowing a handful of extra possible characters to a set of some 60 possibilities is practically irrelevant as long as there is a nice password length.

So we basically agree, I guess. Peace. And your bug report is a good one.

My only interest in joining this discussion is not directed at you, but at whoever aims to fix this. I just think that between these two options...

Possible Fix

prohibit the usage of some special characters at the password input field (less secure) OR escape the password correctly (more secure)

... we should choose the second. That's the whole point I've been (clumsily) trying to make.