From 9a1d16fe97abec49815bfe79055e1516505ceea0 Mon Sep 17 00:00:00 2001 From: terrafrost Date: Sat, 18 Jun 2022 21:40:12 -0500 Subject: [PATCH] ASN1: make it so that null is returned if the BER can't be decoded --- phpseclib/Crypt/Common/Formats/Keys/PKCS8.php | 27 ++++++++++++++-- phpseclib/Crypt/DH/Formats/Keys/PKCS1.php | 2 +- phpseclib/Crypt/DH/Formats/Keys/PKCS8.php | 3 +- phpseclib/Crypt/DSA/Formats/Keys/PKCS1.php | 2 +- phpseclib/Crypt/DSA/Formats/Keys/PKCS8.php | 2 +- phpseclib/Crypt/EC/Formats/Keys/PKCS1.php | 6 ++-- phpseclib/Crypt/EC/Formats/Keys/PKCS8.php | 6 ++++ phpseclib/Crypt/RSA/Formats/Keys/PKCS1.php | 2 +- phpseclib/File/ASN1.php | 10 +++--- phpseclib/File/X509.php | 31 ++++++++++++++++--- 10 files changed, 70 insertions(+), 21 deletions(-) diff --git a/phpseclib/Crypt/Common/Formats/Keys/PKCS8.php b/phpseclib/Crypt/Common/Formats/Keys/PKCS8.php index e0b35820..11d99567 100644 --- a/phpseclib/Crypt/Common/Formats/Keys/PKCS8.php +++ b/phpseclib/Crypt/Common/Formats/Keys/PKCS8.php @@ -344,12 +344,15 @@ abstract class PKCS8 extends PKCS $meta['meta']['algorithm'] = $algorithm; $temp = ASN1::decodeBER($decrypted['encryptionAlgorithm']['parameters']); + if (!$temp) { + throw new \RuntimeException('Unable to decode BER'); + } extract(ASN1::asn1map($temp[0], Maps\PBEParameter::MAP)); $iterationCount = (int) $iterationCount->toString(); $cipher->setPassword($password, $kdf, $hash, $salt, $iterationCount); $key = $cipher->decrypt($decrypted['encryptedData']); $decoded = ASN1::decodeBER($key); - if (empty($decoded)) { + if (!$decoded) { throw new \RuntimeException('Unable to decode BER 2'); } @@ -358,6 +361,9 @@ abstract class PKCS8 extends PKCS $meta['meta']['algorithm'] = $algorithm; $temp = ASN1::decodeBER($decrypted['encryptionAlgorithm']['parameters']); + if (!$temp) { + throw new \RuntimeException('Unable to decode BER'); + } $temp = ASN1::asn1map($temp[0], Maps\PBES2params::MAP); extract($temp); @@ -365,6 +371,9 @@ abstract class PKCS8 extends PKCS $meta['meta']['cipher'] = $encryptionScheme['algorithm']; $temp = ASN1::decodeBER($decrypted['encryptionAlgorithm']['parameters']); + if (!$temp) { + throw new \RuntimeException('Unable to decode BER'); + } $temp = ASN1::asn1map($temp[0], Maps\PBES2params::MAP); extract($temp); @@ -372,6 +381,9 @@ abstract class PKCS8 extends PKCS $cipher->setIV($encryptionScheme['parameters']['octetString']); } else { $temp = ASN1::decodeBER($encryptionScheme['parameters']); + if (!$temp) { + throw new \RuntimeException('Unable to decode BER'); + } extract(ASN1::asn1map($temp[0], Maps\RC2CBCParameter::MAP)); $effectiveKeyLength = (int) $rc2ParametersVersion->toString(); switch ($effectiveKeyLength) { @@ -394,6 +406,9 @@ abstract class PKCS8 extends PKCS switch ($keyDerivationFunc['algorithm']) { case 'id-PBKDF2': $temp = ASN1::decodeBER($keyDerivationFunc['parameters']); + if (!$temp) { + throw new \RuntimeException('Unable to decode BER'); + } $prf = ['algorithm' => 'id-hmacWithSHA1']; $params = ASN1::asn1map($temp[0], Maps\PBKDF2params::MAP); extract($params); @@ -412,7 +427,7 @@ abstract class PKCS8 extends PKCS $cipher->setPassword(...$params); $key = $cipher->decrypt($decrypted['encryptedData']); $decoded = ASN1::decodeBER($key); - if (empty($decoded)) { + if (!$decoded) { throw new \RuntimeException('Unable to decode BER 3'); } break; @@ -647,7 +662,7 @@ abstract class PKCS8 extends PKCS } $decoded = ASN1::decodeBER($key); - if (empty($decoded)) { + if (!$decoded) { throw new \RuntimeException('Unable to decode BER'); } @@ -671,12 +686,18 @@ abstract class PKCS8 extends PKCS if ($r['encryptionAlgorithm']['algorithm'] == 'id-PBES2') { $decoded = ASN1::decodeBER($r['encryptionAlgorithm']['parameters']->element); + if (!$decoded) { + throw new \RuntimeException('Unable to decode BER'); + } $r['encryptionAlgorithm']['parameters'] = ASN1::asn1map($decoded[0], ASN1\Maps\PBES2params::MAP); $kdf = &$r['encryptionAlgorithm']['parameters']['keyDerivationFunc']; switch ($kdf['algorithm']) { case 'id-PBKDF2': $decoded = ASN1::decodeBER($kdf['parameters']->element); + if (!$decoded) { + throw new \RuntimeException('Unable to decode BER'); + } $kdf['parameters'] = ASN1::asn1map($decoded[0], Maps\PBKDF2params::MAP); } } diff --git a/phpseclib/Crypt/DH/Formats/Keys/PKCS1.php b/phpseclib/Crypt/DH/Formats/Keys/PKCS1.php index ccf5b3c0..65a0a5db 100644 --- a/phpseclib/Crypt/DH/Formats/Keys/PKCS1.php +++ b/phpseclib/Crypt/DH/Formats/Keys/PKCS1.php @@ -45,7 +45,7 @@ abstract class PKCS1 extends Progenitor $key = parent::load($key, $password); $decoded = ASN1::decodeBER($key); - if (empty($decoded)) { + if (!$decoded) { throw new \RuntimeException('Unable to decode BER'); } diff --git a/phpseclib/Crypt/DH/Formats/Keys/PKCS8.php b/phpseclib/Crypt/DH/Formats/Keys/PKCS8.php index 2e4c26a2..dc5375f2 100644 --- a/phpseclib/Crypt/DH/Formats/Keys/PKCS8.php +++ b/phpseclib/Crypt/DH/Formats/Keys/PKCS8.php @@ -90,8 +90,7 @@ abstract class PKCS8 extends Progenitor $decoded = ASN1::decodeBER($key[$type]); switch (true) { - case empty($decoded): - case !is_array($decoded): + case !isset($decoded): case !isset($decoded[0]['content']): case !$decoded[0]['content'] instanceof BigInteger: throw new \RuntimeException('Unable to decode BER of parameters'); diff --git a/phpseclib/Crypt/DSA/Formats/Keys/PKCS1.php b/phpseclib/Crypt/DSA/Formats/Keys/PKCS1.php index a7208a04..43ef3561 100644 --- a/phpseclib/Crypt/DSA/Formats/Keys/PKCS1.php +++ b/phpseclib/Crypt/DSA/Formats/Keys/PKCS1.php @@ -52,7 +52,7 @@ abstract class PKCS1 extends Progenitor $key = parent::load($key, $password); $decoded = ASN1::decodeBER($key); - if (empty($decoded)) { + if (!$decoded) { throw new \RuntimeException('Unable to decode BER'); } diff --git a/phpseclib/Crypt/DSA/Formats/Keys/PKCS8.php b/phpseclib/Crypt/DSA/Formats/Keys/PKCS8.php index aa0efa68..a5858b7e 100644 --- a/phpseclib/Crypt/DSA/Formats/Keys/PKCS8.php +++ b/phpseclib/Crypt/DSA/Formats/Keys/PKCS8.php @@ -84,7 +84,7 @@ abstract class PKCS8 extends Progenitor } $decoded = ASN1::decodeBER($key[$type . 'Algorithm']['parameters']->element); - if (empty($decoded)) { + if (!$decoded) { throw new \RuntimeException('Unable to decode BER of parameters'); } $components = ASN1::asn1map($decoded[0], Maps\DSAParams::MAP); diff --git a/phpseclib/Crypt/EC/Formats/Keys/PKCS1.php b/phpseclib/Crypt/EC/Formats/Keys/PKCS1.php index 51a6d4ea..d809f4f2 100644 --- a/phpseclib/Crypt/EC/Formats/Keys/PKCS1.php +++ b/phpseclib/Crypt/EC/Formats/Keys/PKCS1.php @@ -66,7 +66,7 @@ abstract class PKCS1 extends Progenitor preg_match('#-*BEGIN EC PRIVATE KEY-*[^-]*-*END EC PRIVATE KEY-*#s', $key, $matches); $decoded = parent::load($matches[0], $password); $decoded = ASN1::decodeBER($decoded); - if (empty($decoded)) { + if (!$decoded) { throw new \RuntimeException('Unable to decode BER'); } @@ -82,7 +82,7 @@ abstract class PKCS1 extends Progenitor preg_match('#-*BEGIN EC PARAMETERS-*[^-]*-*END EC PARAMETERS-*#s', $key, $matches); $decoded = parent::load($matches[0], ''); $decoded = ASN1::decodeBER($decoded); - if (empty($decoded)) { + if (!$decoded) { throw new \RuntimeException('Unable to decode BER'); } $ecParams = ASN1::asn1map($decoded[0], Maps\ECParameters::MAP); @@ -113,7 +113,7 @@ abstract class PKCS1 extends Progenitor $key = parent::load($key, $password); $decoded = ASN1::decodeBER($key); - if (empty($decoded)) { + if (!$decoded) { throw new \RuntimeException('Unable to decode BER'); } diff --git a/phpseclib/Crypt/EC/Formats/Keys/PKCS8.php b/phpseclib/Crypt/EC/Formats/Keys/PKCS8.php index 54a3e96f..5f65421b 100644 --- a/phpseclib/Crypt/EC/Formats/Keys/PKCS8.php +++ b/phpseclib/Crypt/EC/Formats/Keys/PKCS8.php @@ -98,6 +98,9 @@ abstract class PKCS8 extends Progenitor } $decoded = ASN1::decodeBER($key[$type . 'Algorithm']['parameters']->element); + if (!$decoded) { + throw new \RuntimeException('Unable to decode BER'); + } $params = ASN1::asn1map($decoded[0], Maps\ECParameters::MAP); if (!$params) { throw new \RuntimeException('Unable to decode the parameters using Maps\ECParameters'); @@ -113,6 +116,9 @@ abstract class PKCS8 extends Progenitor } $decoded = ASN1::decodeBER($key['privateKey']); + if (!$decoded) { + throw new \RuntimeException('Unable to decode BER'); + } $key = ASN1::asn1map($decoded[0], Maps\ECPrivateKey::MAP); if (isset($key['parameters']) && $params != $key['parameters']) { throw new \RuntimeException('The PKCS8 parameter field does not match the private key parameter field'); diff --git a/phpseclib/Crypt/RSA/Formats/Keys/PKCS1.php b/phpseclib/Crypt/RSA/Formats/Keys/PKCS1.php index b35c7adb..276c6024 100644 --- a/phpseclib/Crypt/RSA/Formats/Keys/PKCS1.php +++ b/phpseclib/Crypt/RSA/Formats/Keys/PKCS1.php @@ -59,7 +59,7 @@ abstract class PKCS1 extends Progenitor $key = parent::load($key, $password); $decoded = ASN1::decodeBER($key); - if (empty($decoded)) { + if (!$decoded) { throw new \RuntimeException('Unable to decode BER'); } diff --git a/phpseclib/File/ASN1.php b/phpseclib/File/ASN1.php index 39a0dc0d..7ae61180 100644 --- a/phpseclib/File/ASN1.php +++ b/phpseclib/File/ASN1.php @@ -191,7 +191,7 @@ abstract class ASN1 * Serves a similar purpose to openssl's asn1parse * * @param Element|string $encoded - * @return array + * @return ?array */ public static function decodeBER($encoded) { @@ -201,10 +201,12 @@ abstract class ASN1 self::$encoded = $encoded; - $decoded = [self::decode_ber($encoded)]; + $decoded = self::decode_ber($encoded); + if ($decoded === false) { + return null; + } - // encapsulate in an array for BC with the old decodeBER - return $decoded; + return [self::decode_ber($encoded)]; } /** diff --git a/phpseclib/File/X509.php b/phpseclib/File/X509.php index 88aedf08..54e5bb36 100644 --- a/phpseclib/File/X509.php +++ b/phpseclib/File/X509.php @@ -468,7 +468,7 @@ class X509 $decoded = ASN1::decodeBER($cert); - if (!empty($decoded)) { + if ($decoded) { $x509 = ASN1::asn1map($decoded[0], Maps\Certificate::MAP); } if (!isset($x509) || $x509 === false) { @@ -593,6 +593,9 @@ class X509 [static::class, 'decodeNameConstraintIP'] : [static::class, 'decodeIP']; $decoded = ASN1::decodeBER($value); + if (!$decoded) { + continue; + } $mapped = ASN1::asn1map($decoded[0], $map, ['iPAddress' => $decoder]); $value = $mapped === false ? $decoded[0] : $mapped; @@ -607,6 +610,9 @@ class X509 $subvalue = &$value[$j]['policyQualifiers'][$k]['qualifier']; if ($map !== false) { $decoded = ASN1::decodeBER($subvalue); + if (!$decoded) { + continue; + } $mapped = ASN1::asn1map($decoded[0], $map); $subvalue = $mapped === false ? $decoded[0] : $mapped; } @@ -722,6 +728,9 @@ class X509 $value = ASN1::encodeDER($values[$j], Maps\AttributeValue::MAP); $decoded = ASN1::decodeBER($value); if (!is_bool($map)) { + if (!$decoded) { + continue; + } $mapped = ASN1::asn1map($decoded[0], $map); if ($mapped !== false) { $values[$j] = $mapped; @@ -771,6 +780,9 @@ class X509 if (!is_bool($map)) { $temp = ASN1::encodeDER($values[$j], $map); $decoded = ASN1::decodeBER($temp); + if (!$decoded) { + continue; + } $values[$j] = ASN1::asn1map($decoded[0], Maps\AttributeValue::MAP); } } @@ -799,6 +811,9 @@ class X509 $map = $this->getMapping($type); if (!is_bool($map)) { $decoded = ASN1::decodeBER($value); + if (!$decoded) { + continue; + } $value = ASN1::asn1map($decoded[0], $map); } } @@ -1721,6 +1736,9 @@ class X509 $map = $this->getMapping($propName); if (!is_bool($map)) { $decoded = ASN1::decodeBER($v); + if (!$decoded) { + return false; + } $v = ASN1::asn1map($decoded[0], $map); } } @@ -2182,7 +2200,7 @@ class X509 $decoded = ASN1::decodeBER($csr); - if (empty($decoded)) { + if (!$decoded) { $this->currentCert = false; return false; } @@ -2295,7 +2313,7 @@ class X509 $decoded = ASN1::decodeBER($spkac); - if (empty($decoded)) { + if (!$decoded) { $this->currentCert = false; return false; } @@ -2393,7 +2411,7 @@ class X509 $decoded = ASN1::decodeBER($crl); - if (empty($decoded)) { + if (!$decoded) { $this->currentCert = false; return false; } @@ -3610,7 +3628,7 @@ class X509 case $key instanceof Element: // Assume the element is a bitstring-packed key. $decoded = ASN1::decodeBER($key->element); - if (empty($decoded)) { + if (!$decoded) { return false; } $raw = ASN1::asn1map($decoded[0], ['type' => ASN1::TYPE_BIT_STRING]); @@ -3669,6 +3687,9 @@ class X509 $publicKey = base64_decode(preg_replace('#-.+-|[\r\n]#', '', $this->publicKey->toString($format))); $decoded = ASN1::decodeBER($publicKey); + if (!$decoded) { + return false; + } $mapped = ASN1::asn1map($decoded[0], Maps\SubjectPublicKeyInfo::MAP); if (!is_array($mapped)) { return false;