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

Disallowing installation of current magento/magento2ce versions due to 0-day in crypt component #161

Conversation

Ocramius
Copy link
Contributor

While it saddens me to report a vulnerability that prevents installation of ALL Magento CE versions, that's what happens with 0-day disclosures. I don't agree with what was done with the original disclosure, I'm just here to add it to the database.

The file provided here will have to be updated once the Magento team has released a replacement for the current crypto.

Ref: http://www.openwall.com/lists/oss-security/2016/07/19/3

Copying disclosure text for reference:

Date: Tue, 19 Jul 2016 00:56:58 -0400
From: Scott Arciszewski <scott@...agonie.com>
To: fulldisclosure@...lists.org, oss-security@...ts.openwall.com
Subject: Ruining the Magic of Magento's Encryption Library

Hello mcrypt, my old friend
I've come to exploit you again
Because a version slowly rotting
Is well-deserved for a boycotting
And the S-box that was planted in its GOST
Still remains
Within the sound of silence

~ 8< ~ 8< ~ 8< ~ 8< ~ 8< ~ 8< ~ 8< ~ 8< ~ 8< ~ 8< ~ 8< ~ 8< ~ 8< ~ 8< ~ 8< ~

Let's talk about Magento.

The Wikipedia page for Magento begins, "Magento is an open-source
e-commerce platform written in PHP." This bears emphasis: e-commerce
platform.

When I hear e-commerce, I think "financial information". I think "credit
card numbers" and "probably PCI-DSS violations should anything be obviously
stupid".

Let's look at how Magento implements cryptography, with a series of
exhibits followed by an explanation of what's happening and why it's
dangerous:

  A.
https://github.com/magento/magento2/blob/6ea7d2d85cded3fa0fbcf4e7aa0dcd4edbf568a6/lib/internal/Magento/Framework/Encryption/Encryptor.php#L268-L320
  B.
https://github.com/magento/magento2/blob/6ea7d2d85cded3fa0fbcf4e7aa0dcd4edbf568a6/lib/internal/Magento/Framework/Encryption/Encryptor.php#L390-L399
  C.
https://github.com/magento/magento2/blob/6ea7d2d85cded3fa0fbcf4e7aa0dcd4edbf568a6/lib/internal/Magento/Framework/Encryption/Crypt.php#L63-L77

D.
https://github.com/magento/magento2/blob/6ea7d2d85cded3fa0fbcf4e7aa0dcd4edbf568a6/lib/internal/Magento/Framework/Encryption/Encryptor.php#L170

If you looked at the code, I promise this is every bit as bad as it looks
at a glance.

EXHIBIT A
=========

Magento's decryption expects up to 4 strings concatenated by a : character.
Depending on the number of pieces, it assumes a totally different setup:

1 piece: Blowfish, in ECB mode!
2 or 3 pieces: Probably blowfish, but maybe AES or Rijndael-256, depending
on the integer supplied by the attacker.
4 pieces: We finally get an initialization vector, which means CBC mode can
be used.

At no point do they authenticate _anything_, so no matter what:

- You get to control which branch is selected by breaking pieces off the
attacker-chosen message.
- You get to choose the ciphertext that the attempted decryption is
performed upon.

EXHIBIT B
=========

If you thought the ability to be encrypted with AES was a saving grace, too
bad. They hard-code your choice to ECB mode.

The only way you can get CBC mode (which, again, is unauthenticated) is to
use the non-standard Rijndael256 cipher.

EXHIBIT C
=========

If you thought it couldn't possibly get any worse, Magento's encryption
library will either:

- Give you an IV consisting entirely of NULL bytes.
- Generate it, using rand(), on a 62-character keyspace.

(Y'know, because it's not XORed with the plaintext in CBC mode and biases
aren't a concern or anything.)

EXHIBIT D
=========

Yes, that is how Magento hashes passwords. Which is weird: They go out of
their way to compare strings in constant-time, but

PUTTING IT ALL TOGETHER
=======================

An attacker has a great deal of control over the ciphertext, and
incidentally which cipher mode is used by the decryption routine.
Nothing is authenticated. At all.
ECB mode everywhere.
When CBC mode is actually used, it's used with a laughably weak IV and a
non-standard cipher. Also, unauthenticated.

Magento, one of the largest open source e-commerce platforms, ships a
broken cryptography library that clueless developers are probably using to
encrypt your credit card information for their client's customers.

Given the prevalence of ECB mode, and the weak IV used in CBC mode, you
should assume anything you encrypted with Magento's encryption library is
both:

- Decryptable, if an attacker can alter plaintexts or ciphertexts and study
the output of either operation, without the key
- Forgeable

This cryptography implementation is very irresponsible and, because
cryptography is involved, warrants immediate full disclosure so everyone
can cease to use their broken crypto as soon as possible.

If you need a remediation strategy, I've got you covered:
https://paragonie.com/blog/2015/11/choosing-right-cryptography-library-for-your-php-project-guide

Scott Arciszewski
Chief Development Officer
Paragon Initiative Enterprises <https://paragonie.com>

… to 0-day in crypt component

Ref: http://www.openwall.com/lists/oss-security/2016/07/19/3

Copying disclosure text for reference:

```
Date: Tue, 19 Jul 2016 00:56:58 -0400
From: Scott Arciszewski <scott@...agonie.com>
To: fulldisclosure@...lists.org, oss-security@...ts.openwall.com
Subject: Ruining the Magic of Magento's Encryption Library

Hello mcrypt, my old friend
I've come to exploit you again
Because a version slowly rotting
Is well-deserved for a boycotting
And the S-box that was planted in its GOST
Still remains
Within the sound of silence

~ 8< ~ 8< ~ 8< ~ 8< ~ 8< ~ 8< ~ 8< ~ 8< ~ 8< ~ 8< ~ 8< ~ 8< ~ 8< ~ 8< ~ 8< ~

Let's talk about Magento.

The Wikipedia page for Magento begins, "Magento is an open-source
e-commerce platform written in PHP." This bears emphasis: e-commerce
platform.

When I hear e-commerce, I think "financial information". I think "credit
card numbers" and "probably PCI-DSS violations should anything be obviously
stupid".

Let's look at how Magento implements cryptography, with a series of
exhibits followed by an explanation of what's happening and why it's
dangerous:

  A.
https://github.com/magento/magento2/blob/6ea7d2d85cded3fa0fbcf4e7aa0dcd4edbf568a6/lib/internal/Magento/Framework/Encryption/Encryptor.php#L268-L320
  B.
https://github.com/magento/magento2/blob/6ea7d2d85cded3fa0fbcf4e7aa0dcd4edbf568a6/lib/internal/Magento/Framework/Encryption/Encryptor.php#L390-L399
  C.
https://github.com/magento/magento2/blob/6ea7d2d85cded3fa0fbcf4e7aa0dcd4edbf568a6/lib/internal/Magento/Framework/Encryption/Crypt.php#L63-L77

D.
https://github.com/magento/magento2/blob/6ea7d2d85cded3fa0fbcf4e7aa0dcd4edbf568a6/lib/internal/Magento/Framework/Encryption/Encryptor.php#L170

If you looked at the code, I promise this is every bit as bad as it looks
at a glance.

EXHIBIT A
=========

Magento's decryption expects up to 4 strings concatenated by a : character.
Depending on the number of pieces, it assumes a totally different setup:

1 piece: Blowfish, in ECB mode!
2 or 3 pieces: Probably blowfish, but maybe AES or Rijndael-256, depending
on the integer supplied by the attacker.
4 pieces: We finally get an initialization vector, which means CBC mode can
be used.

At no point do they authenticate _anything_, so no matter what:

- You get to control which branch is selected by breaking pieces off the
attacker-chosen message.
- You get to choose the ciphertext that the attempted decryption is
performed upon.

EXHIBIT B
=========

If you thought the ability to be encrypted with AES was a saving grace, too
bad. They hard-code your choice to ECB mode.

The only way you can get CBC mode (which, again, is unauthenticated) is to
use the non-standard Rijndael256 cipher.

EXHIBIT C
=========

If you thought it couldn't possibly get any worse, Magento's encryption
library will either:

- Give you an IV consisting entirely of NULL bytes.
- Generate it, using rand(), on a 62-character keyspace.

(Y'know, because it's not XORed with the plaintext in CBC mode and biases
aren't a concern or anything.)

EXHIBIT D
=========

Yes, that is how Magento hashes passwords. Which is weird: They go out of
their way to compare strings in constant-time, but

PUTTING IT ALL TOGETHER
=======================

An attacker has a great deal of control over the ciphertext, and
incidentally which cipher mode is used by the decryption routine.
Nothing is authenticated. At all.
ECB mode everywhere.
When CBC mode is actually used, it's used with a laughably weak IV and a
non-standard cipher. Also, unauthenticated.

Magento, one of the largest open source e-commerce platforms, ships a
broken cryptography library that clueless developers are probably using to
encrypt your credit card information for their client's customers.

Given the prevalence of ECB mode, and the weak IV used in CBC mode, you
should assume anything you encrypted with Magento's encryption library is
both:

- Decryptable, if an attacker can alter plaintexts or ciphertexts and study
the output of either operation, without the key
- Forgeable

This cryptography implementation is very irresponsible and, because
cryptography is involved, warrants immediate full disclosure so everyone
can cease to use their broken crypto as soon as possible.

If you need a remediation strategy, I've got you covered:
https://paragonie.com/blog/2015/11/choosing-right-cryptography-library-for-your-php-project-guide

Scott Arciszewski
Chief Development Officer
Paragon Initiative Enterprises <https://paragonie.com>
```
@Ocramius Ocramius changed the title Disallowing installation of current magento/magento2ce versions dueto 0-day in crypt component Disallowing installation of current magento/magento2ce versions due to 0-day in crypt component Jul 20, 2016
@ocramleznem
Copy link
Contributor

I think it should be reference: composer://magento/community-edition.

@Ocramius
Copy link
Contributor Author

@ocramleznem
Copy link
Contributor

I've seen it, too. I'm a bit confused about that, because on packagist it is registered with magento/community-edition and this is what the validator.php is checking.

    $packagistUrl = sprintf('https://packagist.org/packages/%s.json', $composerPackage);

    if(404 == explode(' ', get_headers($packagistUrl)[0], 3)[1]) {

@Ocramius
Copy link
Contributor Author

@marcomenzel it's not pushed to packagist, it's a locally installable app...

@fabpot fabpot merged commit 54ff469 into FriendsOfPHP:master Jul 20, 2016
@kelunik
Copy link
Contributor

kelunik commented Jul 20, 2016

This one fails the tests now.

@Ocramius Ocramius deleted the add-2016-07-19-openwall-magento2ce-crypto-0day branch July 20, 2016 18:34
@Ocramius
Copy link
Contributor Author

Yeah, that's because of what @marcomenzel reported. Not sure if we should fix it or build an exception for it...

@ocramleznem
Copy link
Contributor

In general there should be decided to include non-packagist-Packages or not. Following the present ruleset there should be a second kind of unqiue reference implemented which could be used for all non-packagist projects like magento2ce.

@ocramleznem
Copy link
Contributor

Seems like the decision has already been made #127

@kelunik
Copy link
Contributor

kelunik commented Jul 20, 2016

@marcomenzel That one is still open, so no decision yet.

@ocramleznem
Copy link
Contributor

Yes, still open, but at least it seems appreciated to implement it.

@fabpot
Copy link
Member

fabpot commented Jul 21, 2016

In the meantime, I think having an exception in the validator is the way to go.

ocramleznem pushed a commit to ocramleznem/security-advisories that referenced this pull request Jul 21, 2016
…ce package which is not provided by packagist
ocramleznem pushed a commit to ocramleznem/security-advisories that referenced this pull request Dec 3, 2016
…ce package which is not provided by packagist
@barryvdh
Copy link
Contributor

barryvdh commented Jul 3, 2018

So is this relevant? It's blocking all M2.0 and 2.1 versions currently (not 2.2), but does make it impossible to check for for the listed security updates like these: https://magento.com/security/patches/magento-2.2.5-and-2.1.14-security-update

Magento itself does not appear to consider this a security flaw, but it's roadmapped for 2.3; magento/magento2#5701

@Ocramius
Copy link
Contributor Author

Ocramius commented Jul 3, 2018

It's blocking all M2.0 and 2.1 versions

This is correct and expected, as I've written at the very beginning of the OP.

Magento itself does not appear to consider this a security flaw, but it's roadmapped for 2.3; magento/magento2#5701

This is to be brought to attention to Magento itself, not here. What happens here is simply excluding ranges that are marked "insecure" due to known security issues being reported.

@barryvdh
Copy link
Contributor

barryvdh commented Jul 3, 2018

Well in that case you should also block version 2.2, but it does make this library unfortunately unusable for Magento2, so imho it might be worth consedering to use the magento security advisories instead.

Ocramius added a commit to Ocramius/security-advisories that referenced this pull request Jul 3, 2018
@Ocramius
Copy link
Contributor Author

Ocramius commented Jul 3, 2018

in that case you should also block version 2.2

#302

imho it might be worth consedering to use the magento security advisories instead.

It doesn't matter what Magento (the company) says here: you have a security advisory by @paragonie-scott here, and that's what counts.

@barryvdh did Magento release a fix for the security issue?

  1. yes -> fix the range to account for the fixes
  2. no -> do not use that version or apply a hotfix

It is this simple.

@barryvdh
Copy link
Contributor

barryvdh commented Jul 3, 2018

The CVE itself only mentions the rand() function in the Crypt constructor; https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2016-6485 and http://www.openwall.com/lists/oss-security/2016/07/27/14

This (merged) Pull Request fixes that specific instance: magento/magento2#15017

That is released in 2.2.5, see https://github.com/magento/magento2/blob/2.2.5/lib/internal/Magento/Framework/Encryption/Crypt.php#L78

But I'm not sure about the other issues described in the first email, and if they are proven to be vulnerable, as they do not have a CVE assigned as far as I'm aware. (Only that is listed in https://www.cvedetails.com/vulnerability-list/vendor_id-15393/product_id-36803/Magento-Magento2.html )

@Ocramius
Copy link
Contributor Author

Ocramius commented Jul 3, 2018

You will likely need to talk to @paragonie-scott about whether the currently deployed fixes in 2.2.5 mitigate the issue: I'm not qualified to do that.

fabpot added a commit that referenced this pull request Jul 3, 2018
…magento-2.2-inclusion

Exclude Magento-CE 2.2 as per #161 discussion
@paragonie-scott
Copy link
Contributor

Strictly speaking, the work being done today to get Magento 2.3 to switch to libsodium is the complete fix. Chosen-ciphertext attacks must never be swept under the rug.

However, MITRE only assigned a CVE to the RNG issues, which as noted above, was fixed in 2.2.5.

Since it doesn't really make sense to block installation until the end of 2018, setting the upper limit of the vulnerable range to 2.2.5 (exclusive) makes sense as a temporary answer.

If the encryption vulnerabilities turn out to be exploitable in other components, I have no doubt future CVEs will be issued. However, there's currently no real solution for Magento users (although, as noted above, 2.3 will offer one.)

@barryvdh
Copy link
Contributor

barryvdh commented Jul 3, 2018

Updated in #303

barryvdh pushed a commit to barryvdh/security-advisories that referenced this pull request Jul 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants