evocode/lemonstand-patch

minimal patching

Closed this issue · 3 comments

Hi --- First, thanks for this important patch!

I'm of the "manual" school here, and also trying to do things minimally. A few questions, if you don't mind:

• When I applied this initially, it seemed that all customer passwords were now invalid.
• Is the random_compat lib only being used to generate a single SALTED_COOKIE config variable?
• Would it be possible to simply add SALTED_COOKIE as a constant into the "config/config.php" file?

I'm not clear from the bug report how the vulnerability is achieved; can you elaborate on this a bit without endangering other LS sites? Basically, is the essentially about inadequate salting of passwords?

No problem!

When I applied this initially, it seemed that all customer passwords were now invalid.

We don't want to change any existing authentication tokens or salts. We are introducing a new one called COOKIE_SALT that is used only for the cookie token generation to invalidate all existing admin cookies. It should not invalidate any passwords unless you altered an existing token.

Is the random_compat lib only being used to generate a single SALTED_COOKIE config variable? Would it be possible to simply add SALTED_COOKIE as a constant into the "config/config.php" file?

Yes. If you want to generate one manually you can add it directly to config.php.

I'm not clear from the bug report how the vulnerability is achieved; can you elaborate on this a bit without endangering other LS sites? Basically, is the essentially about inadequate salting of passwords?

I can't expand to much as it would reveal the process to hack the sites. There are two problems: One, they can download any file which they used to target the configuration files. Two, because they grabbed those files and LS has a design flaw (it uses the same decryption keys for the dat that are the same for everyone) they generate an authorization token to log into the admin.

The solution then was to kill the method they were gaining access to the files. Then change the authentication token generation to use a new salt since the old ones leaked and we can't change existing tokens without invalidating any existing data.

I see where my problem was: I had existing cookies in my browser, so no matter what I did, since the salted_cookie function does this...

if ($salt_key)
return md5($salt_key);

if (strlen($this->salt_cookie))
return $this->salt_cookie;
...

My old cookies were confusing the system. Does that make sense?

It should have probably logged you out as soon as you added the salt_cookie. But maybe something happened in between requests that it authenticated you with the old one while also creating a new one. I should probably add a step to make sure you are logged out before making the changes.