Different inputs can result in the same password

[daffy1234]

lesspass/core/src/lesspass.js

Line 61 in 1e6cec0

profile.site + profile.login + profile.options.counter.toString(16);

Simply concatenating these two values can lead to different inputs becoming the same output. For example, site name "example.co" and username ".ukraine" will result in the same passwords as "example.co.uk" and "raine".

[SoftwUser]

Correct observation in the more or less unlikely case of exact matching variables (options, length, counter). Do you consider that as being problematic?

[panther2]

@daffy1234 In case that constructed situation occured to you in reality, you could bypass it, by simply giving different (for example) counters, if it really bothered you.

[daffy1234]

I bring it up because it breaks cryptographic principles.

The average user won't care about the extra options, and will only care about the 3 main options (site, username, password). Though it gets even more practical of an issue when you take the counter into consideration.

Imagine someone signing up with the username "Albert" with default settings. Then their password is breached so they change the counter to 17 (11 in hex). Then this same person makes an alternate account, again with default settings, under the username "Albert1". The simply concatenated strings would both end up being "example.comAlbert11".

You can say "well he should use a different counter", but the user has no indication there's a problem.

So do I consider this problematic? Yes.

[xates]

How about separating the different fields with a newline before hashing?

[daffy1234]

@xates If it can be guaranteed that newline characters can't appear in the username or site name (which in this case is a reasonable assumption) then that would work.

Another way to go about it is hashing each part individually, concatenating the hashes, then hashing that to get the resulting salt.

[SoftwUser]

That would all be breaking changes.
Though I agree this is a valid issue, it does not justify changes that break all the passwords of existing users, IMHO.

If unlikely cases as outlined really occurred, I would see enough existing options to overcome it.

I mean users have to deal with LessPass' options sooner or later.

[daffy1234]

Yeah, it definitely would be destructive to mess with the hashing formulas.

One way to go about it in a non-breaking way is provide it as a non-default option. Maybe an on/off slider called "Unambiguous hashing" which is off by default. Maybe with a (learn more) link next to it giving more detail.

Hmm but then you get back to the issue of the average user not caring about options. Just brainstorming here.

[SoftwUser]

provide it as a non-default option

Yes, I like that idea.

Hmm but then you get back to the issue of the average user not caring about options.

IMHO: Once a user starts using LessPass he will be forced to care about options sooner or later. And I think "sooner" rather than "later", as the requirement of using options comes in quickly. There still are some websites not accepting every special character, for example. Or there are maybe routines to change the password every two weeks. Or the password my only consist of 8 characters. Just to name some use cases.

When I started using LessPass and needed to change my old passwords to those provided by LessPass, I ran into the need of using options almost instantly.

So maybe it is "only" a question of documentation.

[guillaumevincent]

Yeah, it definitely would be destructive to mess with the hashing formulas.

It will break the existing passwords, and this should be fixed in a version 3. This is a bug but it could be consider not so critical because the condition to create a same password for a user is really small.
I let this issue open to fix this in a new version.

Thank you for raising the issue

[jdeniau]

@daffy1234 @xates A possibly simpler way is to make a JSON object of those value, and then encode this value.

hash({ "username": "xxx", "site": "xxx", "counter": XX, "etc": "..." })

It will be then simpler to add other fields to the object if needed.
You could even imagine an option to select wich fields are encoded (but it will probably make the interface really complex ;) )

[sdaro]

In my opinion there shouldn't be any doubts that this has to be fixed, because @daffy1234 shows us a scenario that is unacceptable in security perspective.

Maybe we should consider to introduce a lesspass algorithm version number and make this as an option available, but not selectable. This value should be saved in the database for every entry separately. Old entries with no version number use the old algorithm version for legacy reasons. For newly generated passwords, the higher version will be used and saved.

It wouldn't be the last time where a conceptional bug was found and a solution with a algorithm change makes it possible to handle upcoming issues as well.

@jdeniau
I like your idea, because it solves the quoting problem once and for all.

[Pizzacus]

@sdaro I think it would be simpler to have a checkbox saying "Use Legacy Algorithm", and show a message to inform everyone of the algorithm change to tell them to either change their passwords or check that box when obtaining their password.

That way, LessPass stays stateless: Nothing is saved and you can re-obtain any previous passwords.

Because relying on saving which passwords were generated with the previous algorithm feels wrong to me.

[edouard-lopez]

@sdaro We already have an version number for the algorithm, only available on the connected mode.
For more info you can have a look at #93, #121 and Harmony's release note for more info.

Has mention earlier, this has been considered as not critical and will be fixed when we release v3 (TBD).

[0x2b3bfa0]

A possibly simpler way is to make a JSON object of those value, and then encode this value.

hash({ "username": "xxx", "site": "xxx", "counter": XX, "etc": "..." })

Even would be possible to hash each field separately and then hash the concatenated results, as suggested @daffy1234. Would JSON imply any issue related to quote escaping and character or datatype representation? To hash a JSON object we need to canonicalize it first (i.e. apply common whitespacing and typing rules).

This would allow to provide a pre-hashed master password from a hardware security token or a trusted platform module without compromising the plaintext. See #387 and #145

[GregoryKogan]

I solved similar problem with prepending string length to every string in their combination. For example, site name "example.co" and username ".israel" will result in

"10:example.co7:.israel"

But "example.co.is" and "rael" will result in a different string

"13:example.co.is4:rael".

Sample code in python:

def _combine_strings(strings: list[str]) -> str:
        return "".join(f"{len(s)}:{s}" for s in strings)

[renatofrota]

Points not mentioned:

1. the generated passwords would end up saved to the database of 2 different websites;
2. if at least 1 of the 2 websites is using salt (what is very likely), hashes will be different;
3. and beside all that, for the generated passwords (unhashed) itself to match, the master password of both users need to be the same as well.

So it's definitely not an actual security issue, IMHO.