Skip to content

Commit 2a65cc6

Browse files
author
Sergey Shvets
committed
MAGETWO-96983: M2.3 – Sodium crypto adapter errors on unexpected input
1 parent 2957731 commit 2a65cc6

File tree

4 files changed

+83
-55
lines changed

4 files changed

+83
-55
lines changed

‎lib/internal/Magento/Framework/Encryption/Adapter/SodiumChachaIetf.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ public function __construct(
3333
*
3434
* @param string $data
3535
* @return string string
36+
* @throws \SodiumException
3637
*/
3738
public function encrypt(string $data): string
3839
{
@@ -65,6 +66,6 @@ public function decrypt(string $data): string
6566
$this->key
6667
);
6768

68-
return $plainText;
69+
return $plainText !== false ? $plainText : '';
6970
}
7071
}

‎lib/internal/Magento/Framework/Encryption/Test/Unit/Adapter/SodiumChachaIetfTest.php

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,17 @@
1111
*/
1212
namespace Magento\Framework\Encryption\Test\Unit\Adapter;
1313

14-
class SodiumChachaIetfTest extends \PHPUnit\Framework\TestCase
14+
use Magento\Framework\Encryption\Adapter\SodiumChachaIetf;
15+
use PHPUnit\Framework\TestCase;
16+
17+
class SodiumChachaIetfTest extends TestCase
1518
{
19+
/**
20+
* @return array
21+
*/
1622
public function getCryptData(): array
1723
{
18-
$fixturesFilename = __DIR__ . '/../Crypt/_files/_sodium_chachaieft_fixtures.php';
19-
20-
$result = include $fixturesFilename;
24+
$result = include __DIR__ . '/../Crypt/_files/_sodium_chachaieft_fixtures.php';
2125
/* Restore encoded string back to binary */
2226
foreach ($result as &$cryptParams) {
2327
$cryptParams['encrypted'] = base64_decode($cryptParams['encrypted']);
@@ -29,21 +33,30 @@ public function getCryptData(): array
2933

3034
/**
3135
* @dataProvider getCryptData
36+
*
37+
* @param string $key
38+
* @param string $encrypted
39+
* @param string $decrypted
40+
* @throws \SodiumException
3241
*/
33-
public function testEncrypt(string $key, string $encrypted, string $decrypted)
42+
public function testEncrypt(string $key, string $encrypted, string $decrypted): void
3443
{
35-
$crypt = new \Magento\Framework\Encryption\Adapter\SodiumChachaIetf($key);
44+
$crypt = new SodiumChachaIetf($key);
3645
$result = $crypt->encrypt($decrypted);
3746

3847
$this->assertNotEquals($encrypted, $result);
3948
}
4049

4150
/**
4251
* @dataProvider getCryptData
52+
*
53+
* @param string $key
54+
* @param string $encrypted
55+
* @param string $decrypted
4356
*/
44-
public function testDecrypt(string $key, string $encrypted, string $decrypted)
57+
public function testDecrypt(string $key, string $encrypted, string $decrypted): void
4558
{
46-
$crypt = new \Magento\Framework\Encryption\Adapter\SodiumChachaIetf($key);
59+
$crypt = new SodiumChachaIetf($key);
4760
$result = $crypt->decrypt($encrypted);
4861

4962
$this->assertEquals($decrypted, $result);

‎lib/internal/Magento/Framework/Encryption/Test/Unit/Crypt/_files/_sodium_chachaieft_fixtures.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,4 +32,14 @@
3232
'encrypted' => 'UglO9dEgslFpwPwejJmrK89PmBicv+I1pfdaXaEI69IrETD8LpdzOLF7',
3333
'decrypted' => 'Hello World!!!',
3434
],
35+
5 => [
36+
'key' => '6wRADHwwCBGgdxbcHhovGB0upmg0mbsN',
37+
'encrypted' => '',
38+
'decrypted' => '',
39+
],
40+
6 => [
41+
'key' => '6wRADHwwCBGgdxbcHhovGB0upmg0mbsN',
42+
'encrypted' => 'bWFsZm9ybWVkLWlucHV0',
43+
'decrypted' => '',
44+
],
3545
];

‎lib/internal/Magento/Framework/Encryption/Test/Unit/EncryptorTest.php

Lines changed: 50 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -8,73 +8,77 @@
88

99
namespace Magento\Framework\Encryption\Test\Unit;
1010

11-
use Magento\Framework\Encryption\Adapter\Mcrypt;
11+
use Magento\Framework\App\DeploymentConfig;
1212
use Magento\Framework\Encryption\Adapter\SodiumChachaIetf;
13-
use Magento\Framework\Encryption\Encryptor;
1413
use Magento\Framework\Encryption\Crypt;
14+
use Magento\Framework\Encryption\Encryptor;
15+
use Magento\Framework\Math\Random;
16+
use PHPUnit\Framework\MockObject\MockObject;
17+
use PHPUnit\Framework\TestCase;
1518

16-
class EncryptorTest extends \PHPUnit\Framework\TestCase
19+
class EncryptorTest extends TestCase
1720
{
18-
const CRYPT_KEY_1 = 'g9mY9KLrcuAVJfsmVUSRkKFLDdUPVkaZ';
19-
const CRYPT_KEY_2 = '7wEjmrliuqZQ1NQsndSa8C8WHvddeEbN';
21+
private const CRYPT_KEY_1 = 'g9mY9KLrcuAVJfsmVUSRkKFLDdUPVkaZ';
22+
private const CRYPT_KEY_2 = '7wEjmrliuqZQ1NQsndSa8C8WHvddeEbN';
2023

2124
/**
22-
* @var \Magento\Framework\Encryption\Encryptor
25+
* @var Encryptor
2326
*/
2427
protected $_model;
2528

2629
/**
27-
* @var \PHPUnit_Framework_MockObject_MockObject
30+
* @var Random | MockObject
2831
*/
2932
protected $_randomGenerator;
3033

3134
protected function setUp()
3235
{
33-
$this->_randomGenerator = $this->createMock(\Magento\Framework\Math\Random::class);
34-
$deploymentConfigMock = $this->createMock(\Magento\Framework\App\DeploymentConfig::class);
35-
$deploymentConfigMock->expects($this->any())
36+
$this->_randomGenerator = $this->createMock(Random::class);
37+
/** @var DeploymentConfig | MockObject $deploymentConfigMock */
38+
$deploymentConfigMock = $this->createMock(DeploymentConfig::class);
39+
$deploymentConfigMock
3640
->method('get')
3741
->with(Encryptor::PARAM_CRYPT_KEY)
38-
->will($this->returnValue(self::CRYPT_KEY_1));
39-
$this->_model = new \Magento\Framework\Encryption\Encryptor($this->_randomGenerator, $deploymentConfigMock);
42+
->willReturn(self::CRYPT_KEY_1);
43+
$this->_model = new Encryptor($this->_randomGenerator, $deploymentConfigMock);
4044
}
4145

42-
public function testGetHashNoSalt()
46+
public function testGetHashNoSalt(): void
4347
{
4448
$this->_randomGenerator->expects($this->never())->method('getRandomString');
4549
$expected = '5e884898da28047151d0e56f8dc6292773603d0d6aabbdd62a11ef721d1542d8';
4650
$actual = $this->_model->getHash('password');
4751
$this->assertEquals($expected, $actual);
4852
}
4953

50-
public function testGetHashSpecifiedSalt()
54+
public function testGetHashSpecifiedSalt(): void
5155
{
5256
$this->_randomGenerator->expects($this->never())->method('getRandomString');
5357
$expected = '13601bda4ea78e55a07b98866d2be6be0744e3866f13c00c811cab608a28f322:salt:1';
5458
$actual = $this->_model->getHash('password', 'salt');
5559
$this->assertEquals($expected, $actual);
5660
}
5761

58-
public function testGetHashRandomSaltDefaultLength()
62+
public function testGetHashRandomSaltDefaultLength(): void
5963
{
6064
$salt = '-----------random_salt----------';
6165
$this->_randomGenerator
6266
->expects($this->once())
6367
->method('getRandomString')
6468
->with(32)
65-
->will($this->returnValue($salt));
69+
->willReturn($salt);
6670
$expected = 'a1c7fc88037b70c9be84d3ad12522c7888f647915db78f42eb572008422ba2fa:' . $salt . ':1';
6771
$actual = $this->_model->getHash('password', true);
6872
$this->assertEquals($expected, $actual);
6973
}
7074

71-
public function testGetHashRandomSaltSpecifiedLength()
75+
public function testGetHashRandomSaltSpecifiedLength(): void
7276
{
7377
$this->_randomGenerator
7478
->expects($this->once())
7579
->method('getRandomString')
7680
->with(11)
77-
->will($this->returnValue('random_salt'));
81+
->willReturn('random_salt');
7882
$expected = '4c5cab8dd00137d11258f8f87b93fd17bd94c5026fc52d3c5af911dd177a2611:random_salt:1';
7983
$actual = $this->_model->getHash('password', 11);
8084
$this->assertEquals($expected, $actual);
@@ -87,7 +91,7 @@ public function testGetHashRandomSaltSpecifiedLength()
8791
*
8892
* @dataProvider validateHashDataProvider
8993
*/
90-
public function testValidateHash($password, $hash, $expected)
94+
public function testValidateHash($password, $hash, $expected): void
9195
{
9296
$actual = $this->_model->validateHash($password, $hash);
9397
$this->assertEquals($expected, $actual);
@@ -96,7 +100,7 @@ public function testValidateHash($password, $hash, $expected)
96100
/**
97101
* @return array
98102
*/
99-
public function validateHashDataProvider()
103+
public function validateHashDataProvider(): array
100104
{
101105
return [
102106
['password', 'hash:salt:1', false],
@@ -111,13 +115,13 @@ public function validateHashDataProvider()
111115
* @dataProvider encryptWithEmptyKeyDataProvider
112116
* @expectedException \SodiumException
113117
*/
114-
public function testEncryptWithEmptyKey($key)
118+
public function testEncryptWithEmptyKey($key): void
115119
{
116-
$deploymentConfigMock = $this->createMock(\Magento\Framework\App\DeploymentConfig::class);
120+
$deploymentConfigMock = $this->createMock(DeploymentConfig::class);
117121
$deploymentConfigMock->expects($this->any())
118122
->method('get')
119123
->with(Encryptor::PARAM_CRYPT_KEY)
120-
->will($this->returnValue($key));
124+
->willReturn($key);
121125
$model = new Encryptor($this->_randomGenerator, $deploymentConfigMock);
122126
$value = 'arbitrary_string';
123127
$this->assertEquals($value, $model->encrypt($value));
@@ -136,13 +140,14 @@ public function encryptWithEmptyKeyDataProvider()
136140
*
137141
* @dataProvider decryptWithEmptyKeyDataProvider
138142
*/
139-
public function testDecryptWithEmptyKey($key)
143+
public function testDecryptWithEmptyKey($key): void
140144
{
141-
$deploymentConfigMock = $this->createMock(\Magento\Framework\App\DeploymentConfig::class);
145+
/** @var DeploymentConfig | MockObject $deploymentConfigMock */
146+
$deploymentConfigMock = $this->createMock(DeploymentConfig::class);
142147
$deploymentConfigMock->expects($this->any())
143148
->method('get')
144149
->with(Encryptor::PARAM_CRYPT_KEY)
145-
->will($this->returnValue($key));
150+
->willReturn($key);
146151
$model = new Encryptor($this->_randomGenerator, $deploymentConfigMock);
147152
$value = 'arbitrary_string';
148153
$this->assertEquals('', $model->decrypt($value));
@@ -151,36 +156,35 @@ public function testDecryptWithEmptyKey($key)
151156
/**
152157
* @return array
153158
*/
154-
public function decryptWithEmptyKeyDataProvider()
159+
public function decryptWithEmptyKeyDataProvider(): array
155160
{
156161
return [[null], [0], [''], ['0']];
157162
}
158163

159-
public function testEncrypt()
164+
public function testEncrypt(): void
160165
{
161166
// sample data to encrypt
162167
$data = 'Mares eat oats and does eat oats, but little lambs eat ivy.';
163168

164169
$actual = $this->_model->encrypt($data);
165170

166171
// Extract the initialization vector and encrypted data
167-
$parts = explode(':', $actual, 3);
168-
list(, , $encryptedData) = $parts;
172+
[, , $encryptedData] = explode(':', $actual, 3);
169173

170174
$crypt = new SodiumChachaIetf(self::CRYPT_KEY_1);
171175
// Verify decrypted matches original data
172176
$this->assertEquals($data, $crypt->decrypt(base64_decode((string)$encryptedData)));
173177
}
174178

175-
public function testDecrypt()
179+
public function testDecrypt(): void
176180
{
177181
$message = 'Mares eat oats and does eat oats, but little lambs eat ivy.';
178182
$encrypted = $this->_model->encrypt($message);
179183

180184
$this->assertEquals($message, $this->_model->decrypt($encrypted));
181185
}
182186

183-
public function testLegacyDecrypt()
187+
public function testLegacyDecrypt(): void
184188
{
185189
// sample data to encrypt
186190
$data = '0:2:z3a4ACpkU35W6pV692U4ueCVQP0m0v0p:' .
@@ -189,26 +193,26 @@ public function testLegacyDecrypt()
189193
$actual = $this->_model->decrypt($data);
190194

191195
// Extract the initialization vector and encrypted data
192-
$parts = explode(':', $data, 4);
193-
list(, , $iv, $encrypted) = $parts;
196+
[, , $iv, $encrypted] = explode(':', $data, 4);
194197

195198
// Decrypt returned data with RIJNDAEL_256 cipher, cbc mode
196199
$crypt = new Crypt(self::CRYPT_KEY_1, MCRYPT_RIJNDAEL_256, MCRYPT_MODE_CBC, $iv);
197200
// Verify decrypted matches original data
198201
$this->assertEquals($encrypted, base64_encode($crypt->encrypt($actual)));
199202
}
200203

201-
public function testEncryptDecryptNewKeyAdded()
204+
public function testEncryptDecryptNewKeyAdded(): void
202205
{
203-
$deploymentConfigMock = $this->createMock(\Magento\Framework\App\DeploymentConfig::class);
206+
/** @var DeploymentConfig | MockObject $deploymentConfigMock */
207+
$deploymentConfigMock = $this->createMock(DeploymentConfig::class);
204208
$deploymentConfigMock->expects($this->at(0))
205209
->method('get')
206210
->with(Encryptor::PARAM_CRYPT_KEY)
207-
->will($this->returnValue(self::CRYPT_KEY_1));
211+
->willReturn(self::CRYPT_KEY_1);
208212
$deploymentConfigMock->expects($this->at(1))
209213
->method('get')
210214
->with(Encryptor::PARAM_CRYPT_KEY)
211-
->will($this->returnValue(self::CRYPT_KEY_1 . "\n" . self::CRYPT_KEY_2));
215+
->willReturn(self::CRYPT_KEY_1 . "\n" . self::CRYPT_KEY_2);
212216
$model1 = new Encryptor($this->_randomGenerator, $deploymentConfigMock);
213217
// simulate an encryption key is being added
214218
$model2 = new Encryptor($this->_randomGenerator, $deploymentConfigMock);
@@ -222,33 +226,33 @@ public function testEncryptDecryptNewKeyAdded()
222226
$this->assertSame($data, $decryptedData, 'Encryptor failed to decrypt data encrypted by old keys.');
223227
}
224228

225-
public function testValidateKey()
229+
public function testValidateKey(): void
226230
{
227231
$this->_model->validateKey(self::CRYPT_KEY_1);
228232
}
229233

230234
/**
231235
* @expectedException \Exception
232236
*/
233-
public function testValidateKeyInvalid()
237+
public function testValidateKeyInvalid(): void
234238
{
235239
$this->_model->validateKey('----- ');
236240
}
237241

238242
/**
239243
* @return array
240244
*/
241-
public function useSpecifiedHashingAlgoDataProvider()
245+
public function useSpecifiedHashingAlgoDataProvider(): array
242246
{
243247
return [
244248
['password', 'salt', Encryptor::HASH_VERSION_MD5,
245-
'67a1e09bb1f83f5007dc119c14d663aa:salt:0'],
249+
'67a1e09bb1f83f5007dc119c14d663aa:salt:0'],
246250
['password', 'salt', Encryptor::HASH_VERSION_SHA256,
247-
'13601bda4ea78e55a07b98866d2be6be0744e3866f13c00c811cab608a28f322:salt:1'],
251+
'13601bda4ea78e55a07b98866d2be6be0744e3866f13c00c811cab608a28f322:salt:1'],
248252
['password', false, Encryptor::HASH_VERSION_MD5,
249-
'5f4dcc3b5aa765d61d8327deb882cf99'],
253+
'5f4dcc3b5aa765d61d8327deb882cf99'],
250254
['password', false, Encryptor::HASH_VERSION_SHA256,
251-
'5e884898da28047151d0e56f8dc6292773603d0d6aabbdd62a11ef721d1542d8']
255+
'5e884898da28047151d0e56f8dc6292773603d0d6aabbdd62a11ef721d1542d8']
252256
];
253257
}
254258

@@ -260,7 +264,7 @@ public function useSpecifiedHashingAlgoDataProvider()
260264
* @param $hashAlgo
261265
* @param $expected
262266
*/
263-
public function testGetHashMustUseSpecifiedHashingAlgo($password, $salt, $hashAlgo, $expected)
267+
public function testGetHashMustUseSpecifiedHashingAlgo($password, $salt, $hashAlgo, $expected): void
264268
{
265269
$hash = $this->_model->getHash($password, $salt, $hashAlgo);
266270
$this->assertEquals($expected, $hash);

0 commit comments

Comments
 (0)