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

Consider removing JCryptCipherSimple #8327

Closed
paragonie-scott opened this Issue Nov 8, 2015 · 3 comments

Comments

Projects
None yet
4 participants
@paragonie-scott
Contributor

paragonie-scott commented Nov 8, 2015

/**
* Method to encrypt a data string.
*
* @param string $data The data string to encrypt.
* @param JCryptKey $key The key[/pair] object to use for encryption.
*
* @return string The encrypted data string.
*
* @since 12.1
* @throws InvalidArgumentException
*/
public function encrypt($data, JCryptKey $key)
{
// Validate key.
if ($key->type != 'simple')
{
throw new InvalidArgumentException('Invalid key of type: ' . $key->type . '. Expected simple.');
}
$encrypted = '';
$tmp = $key->private;
// Split up the input into a character array and get the number of characters.
$chars = preg_split('//', $data, -1, PREG_SPLIT_NO_EMPTY);
$charCount = count($chars);
// Repeat the key as many times as necessary to ensure that the key is at least as long as the input.
for ($i = 0; $i < $charCount; $i = strlen($tmp))
{
$tmp = $tmp . $tmp;
}
// Get the XOR values between the ASCII values of the input and key characters for all input offsets.
for ($i = 0; $i < $charCount; $i++)
{
$encrypted .= $this->_intToHex(ord($tmp[$i]) ^ ord($chars[$i]));
}
return $encrypted;
}

XORing the plaintext with str_repeat($secretkey) is worse than encrypting in ECB mode.

There is no salvaging this "encryption" code. rm it, it's not secure.

@Bakual

This comment has been minimized.

Show comment
Hide comment
@Bakual

Bakual Nov 8, 2015

Contributor

JCryptCipherSimple is already deprecated and will be removed with 4.0.
Due to B/C it is still shiped but it's recommended to use one of the more secure alternatives.

Contributor

Bakual commented Nov 8, 2015

JCryptCipherSimple is already deprecated and will be removed with 4.0.
Due to B/C it is still shiped but it's recommended to use one of the more secure alternatives.

mbabker added a commit to mbabker/joomla-cms that referenced this issue Nov 9, 2015

@paragonie-scott

This comment has been minimized.

Show comment
Hide comment
@paragonie-scott

paragonie-scott Nov 9, 2015

Contributor

That's good, but I would seriously consider throwing an E_NOTICE or E_WARNING whenever it's used as of 3.5.0.

Contributor

paragonie-scott commented Nov 9, 2015

That's good, but I would seriously consider throwing an E_NOTICE or E_WARNING whenever it's used as of 3.5.0.

@wilsonge

This comment has been minimized.

Show comment
Hide comment
@wilsonge

wilsonge Nov 11, 2015

Contributor

We aren't going to place potentially publicly visible notices on people's sites. The deprecation is the best we can do until Joomla 4 (potentially we can shove something in the log files but the reality is that very very few people ever check that file). There are no active use cases of this class in core. So I think we're pretty much doing the best we can at this stage. Our Joomla 4 deprecations branch already has this class (and all use cases removed as well https://github.com/joomla-projects/joomla-pythagoras/tree/feature/deprecations)

Contributor

wilsonge commented Nov 11, 2015

We aren't going to place potentially publicly visible notices on people's sites. The deprecation is the best we can do until Joomla 4 (potentially we can shove something in the log files but the reality is that very very few people ever check that file). There are no active use cases of this class in core. So I think we're pretty much doing the best we can at this stage. Our Joomla 4 deprecations branch already has this class (and all use cases removed as well https://github.com/joomla-projects/joomla-pythagoras/tree/feature/deprecations)

@wilsonge wilsonge closed this Nov 11, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment