Skip to content

Commit

Permalink
X509: don't attempt to parse multi-cert PEMs
Browse files Browse the repository at this point in the history
  • Loading branch information
@terrafrost
terrafrost committed Oct 30, 2020
1 parent 2ae6834 commit 00c9edc
Show file tree
Hide file tree
Showing 2 changed files with 117 additions and 2 deletions.
4 changes: 2 additions & 2 deletions phpseclib/File/X509.php
View file
Expand Up @@ -5147,10 +5147,10 @@ function _extractBER($str)
$temp = strlen($str) <= ini_get('pcre.backtrack_limit') ?
preg_replace('#.*?^-+[^-]+-+[\r\n ]*$#ms', '', $str, 1) :
$str;
// remove the -----BEGIN CERTIFICATE----- and -----END CERTIFICATE----- stuff
$temp = preg_replace('#-+[^-]+-+#', '', $temp);
// remove new lines
$temp = str_replace(array("\r", "\n", ' '), '', $temp);
// remove the -----BEGIN CERTIFICATE----- and -----END CERTIFICATE----- stuff
$temp = preg_replace('#^-+[^-]+-+|-+[^-]+-+$#', '', $temp);
$temp = preg_match('#^[a-zA-Z\d/+]*={0,2}$#', $temp) ? base64_decode($temp) : false;
return $temp != false ? $temp : $str;
}
Expand Down
115 changes: 115 additions & 0 deletions tests/Unit/File/X509/X509Test.php
View file
Expand Up @@ -747,4 +747,119 @@ public function testRandomString()

$this->assertFalse($r);
}

/**
* @group github1542
*/
public function testMultiCertPEM()
{
$a = '-----BEGIN CERTIFICATE-----
MIILODCCCSCgAwIBAgIQDh0LGipJ++wxFLj8X5MXKDANBgkqhkiG9w0BAQsFADCB
kDELMAkGA1UEBhMCVVMxDTALBgNVBAgTBFV0YWgxDTALBgNVBAcTBExlaGkxFzAV
BgNVBAoTDkRpZ2lDZXJ0LCBJbmMuMRkwFwYDVQQLExB3d3cuZGlnaWNlcnQuY29t
MS8wLQYDVQQDEyZEaWdpQ2VydCBWZXJpZmllZCBNYXJrIEludGVybWVkaWF0ZSBD
QTAeFw0yMDA3MzAwMDAwMDBaFw0yMTAxMjUxMjAwMDBaMIIBDjEdMBsGA1UEDxMU
UHJpdmF0ZSBPcmdhbml6YXRpb24xEzARBgsrBgEEAYI3PAIBAxMCVVMxGTAXBgsr
BgEEAYI3PAIBAhMIRGVsYXdhcmUxEDAOBgNVBAUTBzM2MzMwMTkxGTAXBgNVBAkT
EDEwMDAgVyBNYXVkZSBBdmUxDjAMBgNVBBETBTk0MDg1MQswCQYDVQQGEwJVUzET
MBEGA1UECBMKQ2FsaWZvcm5pYTESMBAGA1UEBxMJU3Vubnl2YWxlMR0wGwYDVQQK
ExRMaW5rZWRJbiBDb3Jwb3JhdGlvbjESMBAGCisGAQQBg55fAQMTAlVTMRcwFQYK
KwYBBAGDnl8BBBMHNTY3NTczOTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoC
ggEBAOCl7WccAcvSaf5+pNsV82VjFuwdzEwjDYESZmIuurz95e+JtJZst/M3Hw90
YxKSDV4LdaVFAogXy2F+Npit1KhbBEb8vbBkm4LJ3iM8teE/10JugLyxrcVi3LSj
iKHs+rqxcTJsVYoR+CuPLuAbu4xKi+xQ4tVafrFd0Y21n6OL8nB2SRISHF58kRXq
UDW/NippF1AhcdCc5L5EmXFPCpyWfv+UXgTj9i+/I9AWUC3diHckb5NXd/wS7Jmq
5FE0uixRGTixI5a9uZr0jasTtfhlVtvqFyDmzARB/q9IU0eXm3dtcCJISIXGum6o
yCFUk8pyYsGd/M5Fyw7zbmEqsucCAwEAAaOCBgswggYHMB8GA1UdIwQYMBaAFOsN
zmX0UnV7TbPUsz0w41AYq+NuMB0GA1UdDgQWBBSkuL0+t0wu/+y2xkUY/FOSsiuV
ODAXBgNVHREEEDAOggxsaW5rZWRpbi5jb20wEwYDVR0lBAwwCgYIKwYBBQUHAx8w
gZkGA1UdHwSBkTCBjjBFoEOgQYY/aHR0cDovL2NybDMuZGlnaWNlcnQuY29tL0Rp
Z2lDZXJ0VmVyaWZpZWRNYXJrSW50ZXJtZWRpYXRlQ0EuY3JsMEWgQ6BBhj9odHRw
Oi8vY3JsNC5kaWdpY2VydC5jb20vRGlnaUNlcnRWZXJpZmllZE1hcmtJbnRlcm1l
ZGlhdGVDQS5jcmwwUAYDVR0gBEkwRzA3BglghkgBhv1sCgEwKjAoBggrBgEFBQcC
ARYcaHR0cHM6Ly93d3cuZGlnaWNlcnQuY29tL0NQUzAMBgorBgEEAYOeXwEBMF4G
CCsGAQUFBwEBBFIwUDBOBggrBgEFBQcwAoZCaHR0cDovL2NhY2VydHMuZGlnaWNl
cnQuY29tL0RpZ2lDZXJ0VmVyaWZpZWRNYXJrSW50ZXJtZWRpYXRlQ0EuY3J0MAwG
A1UdEwEB/wQCMAAwggOsBggrBgEFBQcBDASCA54wggOaooIDlqCCA5IwggOOMIID
ijCCA4YWDWltYWdlL3N2Zyt4bWwwIzAhMAkGBSsOAwIaBQAEFGckN8uhuoNkcXXh
wRAm7wkz4JRLMIIDThaCA0pkYXRhOmltYWdlL3N2Zyt4bWw7YmFzZTY0LEg0c0lB
QUFBQUFBQUNsMVR5MjdiTUJDODl5c0k5VXlhKytLanNITElJVWlCRnVqSjkxUlJR
cUdLSGNTQ25PYnJ1NVRVQkMwTVNKN2w3SEozZHJRL3o0OW03bC9PdytuWU51Q3dN
YTlQNC9IY05tV2Fuci9zZHBmTHhWM0luVjRlZCtpOTN5bS9NWmZoZmlwdEUySm9U
T21IeHpKdFlCNzZ5L1hwdFcyODhVWWpab24rdkR2M1AxNU9EOFBZdDgwMEhIL2I1
M056dForR2FleXZ2ZzNIWC8zOTErTit0K0w5ODkxVWpITEh0dm5zWTB6WFd1Ryti
YjVMQkJkek5vR2NSTHdGenk1NzdpeWk4eHlzUXdETDNnR2dnZWlZWTBYczJWQjJu
T0ZRODFQb0hBcVltaFBGUUhLRXVSTDBIdk5CbHhBTGgrQlNsazZwb0R3VXJnUU1i
R3QxcVVBazJwVjlBRS9PQitxc0k1S2xwUlN0cG5GSWxSSlo3RWVDZHZQVzdQNmQ5
T2JtWmgwVFdGd01ZakJrNXlHVnBFc0pNbU1BUjVHS1hmRmhPMzU3cWwwbnNFRGVl
Y29kQmcyVFZVblFjSFJBYkJBMHRDTGRpTDQ4OHRrdVVkZzRkbzF1SExzRlY0cjlD
Q3BsMXRJeDZKemVnMFZ4T2RGeUFTODhMMm03WU1sNnl1QkN5bVpycnNUb2N1YVpS
S094YUZhUXV5UTZGNXJ0cGI3UnUxd1N0SXdPcVV2NkZORjRWdFVqR1dGdEp2NUZn
S3p5d3d4TWprUnVXZFFBZEdEZEJqSjIzdXJGVEdvT0liU3FHRUZhNjc0RGJUQkg0
eTBuOVFBWjBqWHE2WVpDckhZNmlGYlI3cXYwa04rVi8zK0RIdXB2WFdLQXJNcWdF
YThWTXBXbytXRkdVcURPVWkxVXh3M3BJR20yUzZ4VXgyU0FlVUc2V2xGVDUvVnN0
S3FkVitlcWtwNHFTTHFnQlJTcmpnRzFTTlRCb0pETE10ZWpHTWEwT0hyNVg3Q3VZ
cXlPaC9WMFhwNjNJWUxTMTl4YUtlZGx0UHFsWDMzNkEwYlJhNW9nQkFBQTCBigYK
KwYBBAHWeQIEAgR8BHoAeAB2AFVZU64wlgCAbNLrUgimyZ6TGCisEFa0QhxVNhVM
X3WsAAABc6DTF3wAAAQDAEcwRQIgRsnN1miYsyCMT234C14MaMgSAgKHXmc7RrBM
a/1ovTMCIQCOc/THDvltzhZrtnoRSbjc2EYp57A0VVHvduQPa7FKBDANBgkqhkiG
9w0BAQsFAAOCAgEA8UQt5jcUeOaDkhvbLq380Oq1Jy8Vr1BO1GPisn20KRCz/NvE
56f8hhmZlZ1xXfOM+JCaGQnwVwcRBQtLQ/+6bmeT8/WM3hf9A5rP0g0ZxvaAlQtu
e6UjvgnNx02QOKNPrmxN0rW8s24kUi0OAf1ump3SY5Ab+S+ywRG7Ah+3qch+FwA8
CYau9TgV5kvfYDRULBM84EeFhsPcwT+YJ5u7RvkGQobqNao21Ti5tupiks/9NzI8
splBS77Z6bPdFGvZ7pJdXiiDB2+SZdyv8iqDFM6mKRbOcuwAHcTY2zVhcS46H7SO
8OU7L/2y0XQB1rMtQDarCKwdAcsAb2e+N8mYQ0glQX4k41Sf4saMXsU1EjnOCUas
YxvVgJRD+fe4JWf8EO59fElzkrQsT3guBIzV5Kg1dYaCHngCYQIakjKQM0eKxZ3d
vn4648A0vXynhJUThOSxN4jbvVA5uYYHqHDMjJtkBPDA7HtLSIxRNattshOAoeC5
LMszAsL9th/WoXkAa2lTs2kashOHEpx+ncGactrL8tu7dvU01Yk6yP1QAjFEo1Nt
8umUG7jQQIuquB2ry4qzFuQvKpbNQZ//9RsSmq1nni+DEKd/S63N7T8M0FpioLVm
Z2OXFTCG5ORjPUOyMGjxzjEPZWmTOG+gqNOc0HKXbuBAsGZbK4dind+YdZE=
-----END CERTIFICATE-----
-----BEGIN CERTIFICATE-----
MIIHLzCCBRegAwIBAgIQDZXVhKBTvJ0ZjW6meNxHhTANBgkqhkiG9w0BAQsFADCB
iDELMAkGA1UEBhMCVVMxDTALBgNVBAgTBFV0YWgxDTALBgNVBAcTBExlaGkxFzAV
BgNVBAoTDkRpZ2lDZXJ0LCBJbmMuMRkwFwYDVQQLExB3d3cuZGlnaWNlcnQuY29t
MScwJQYDVQQDEx5EaWdpQ2VydCBWZXJpZmllZCBNYXJrIFJvb3QgQ0EwHhcNMTkw
OTIzMTIyNTQyWhcNMzQwOTIzMTIyNTQyWjCBkDELMAkGA1UEBhMCVVMxDTALBgNV
BAgTBFV0YWgxDTALBgNVBAcTBExlaGkxFzAVBgNVBAoTDkRpZ2lDZXJ0LCBJbmMu
MRkwFwYDVQQLExB3d3cuZGlnaWNlcnQuY29tMS8wLQYDVQQDEyZEaWdpQ2VydCBW
ZXJpZmllZCBNYXJrIEludGVybWVkaWF0ZSBDQTCCAiIwDQYJKoZIhvcNAQEBBQAD
ggIPADCCAgoCggIBAPfhMd+2RBjNZpqq0GVUF0kKK72fQxhJbnxgYv7GFpmi69sX
dgeqH9RE07ShTDtkLks9G/GuiXsLEmjSCBDDTwfB3hpbdrZsNQFWOIRHXmU8ykuP
bCd/HVRZGULeWvbt93deEB1el5MpxP9Fs3LKjw7xytbuM/nkGJ4D2R1IHC953FoU
4BYsp+8VB1+7Gh8eKVh+HpmBeEfIB+cuq4FpZKxi+F5J7UjW5yO4SuDcTF4AMY0J
DPuKIy+Og6laNOtDS30P1CUu1N6BwLMYTbeqyYHJ7B3kLWsDceGMqIcxo8zrk1rT
sJctcXHXhB4k2PnVxt8qkQjg2Lo++kU0dFSUyrzvg3WrGypv9vphWMI+vmCjmu2K
0BZLZ4nKshoTX495R6pGbsecGaaGgACB/1NcGI7PVp7spY2ytLvHHZ+Hh446BGFy
AdM8lZCMXEhNftP8RRRVr8mwHHzyIa86r4yk0SkOlXUkNrGdTqqyMSDJ3W7DWGO/
vCObzXiM0aq77ebD/0fE5LsZhEJYx7txF9NA1DoICgHp8zqF35i3UOp4+5IyJ8A9
MjqYcX+LayH06B45bMgTHLmJKcsRYvXAtu+nIvIL0fk4+Ea7kJ7MNx5/udS89b2f
vSFC4hmA3OBDiJqGmldwqJL/HP7RI8O54yj10vpiH4obDo8QgfhKSVoX5HwDAgMB
AAGjggGJMIIBhTAdBgNVHQ4EFgQU6w3OZfRSdXtNs9SzPTDjUBir424wHwYDVR0j
BBgwFoAU7G8ipLME4sFjh+Z3Y+pGaU7u/OswDgYDVR0PAQH/BAQDAgGGMBMGA1Ud
JQQMMAoGCCsGAQUFBwMfMBIGA1UdEwEB/wQIMAYBAf8CAQAwfAYIKwYBBQUHAQEE
cDBuMCQGCCsGAQUFBzABhhhodHRwOi8vb2NzcC5kaWdpY2VydC5jb20wRgYIKwYB
BQUHMAKGOmh0dHA6Ly9jYWNlcnRzLmRpZ2ljZXJ0LmNvbS9EaWdpQ2VydFZlcmlm
aWVkTWFya1Jvb3RDQS5jcnQwSAYDVR0fBEEwPzA9oDugOYY3aHR0cDovL2NybDMu
ZGlnaWNlcnQuY29tL0RpZ2lDZXJ0VmVyaWZpZWRNYXJrUm9vdENBLmNybDBCBgNV
HSAEOzA5MDcGCWCGSAGG/WwKATAqMCgGCCsGAQUFBwIBFhxodHRwczovL3d3dy5k
aWdpY2VydC5jb20vQ1BTMA0GCSqGSIb3DQEBCwUAA4ICAQA6v371ixHAA9WyGlFr
+RvmmqkZZg9pf6+j5sKImeTFAHfYz3TJa2wmDRpZxSRYy9VUFOMPVDEavsJ2i5Ua
jEpkJ/7VHbX60joKBxQHKCbMpbwGen6pTXRaeE6CET2zCMbyiIqT2E6OiBZL8cWT
sgLbdhVKspoOi+c3JwTR4khR24J9IQVxK90Nq3zeciYgBvM3G+ZJtZgkA58CRdex
xVO6O57bwe4ti4rlRgWOGgAFpFrJSD1jqhhHD3MWg17NI4k0ciBPoDHHBAI8hiqg
jPM+y6aEGww7BFSfkp5tl/Aq9uGXCwxNLOc3UlUd8Cc0qH1KfkvjrumVMmJhsk9I
88YefbCuGPZ5Q8V2LIz7LrNDh0VRq/HSENXn1sBGlAFahgUTk/cWJ8nKA+8mHMlE
NGmx205Rtbf9lVL1CbH3QFwlvGEfqoMXw6G9JW2hFQVTKpBKuzjQw43CEw12lstP
oa96ixNAXXVGJsdKpYCqTIWJ0x1DssvG2shvzdHxawvYQ3C+/jaEoQ6bxSIdanI2
NMtBdy9Q0TjDc7uf/eaUYKkP4wskNc1Os23oHllFHVm++8wdDltNulc7B1TXIQ+2
oD5EoULMFSVUHX8gtyd463GgOQtBDwf3aZ4Xe6eDrhdfI/4IW098kVcg+qFO841L
qzFkAKWjJj4KjfrbZX4C0Spfxw==
-----END CERTIFICATE-----';

$x509 = new File_X509();
$r = $x509->loadX509($a);

$this->assertFalse($r);
}
}

3 comments on commit 00c9edc

@terrafrost
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #1542

@LeonMelis
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this fix is correct, neither is the testcase. In fact, this testcase would have passed before the change. This testcase is not the issue in #1542

The problem lies with a certificate chain where the former cert B64 body does not end with a = character. By removing the whitespace and the begin/end headers, you bascially make one big B64 string, which will pass validator, when it shouldn't.

At this point, you are relying on the ASN.1 parser to stop parsing after the first certificate structure.

See example1.txt in #1542 for a correct testcase.

@terrafrost
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The unit test was bad - I used example2.txt instead of example1.txt. The original code fails if I swap out the certs:

terrafrost@7394163
https://travis-ci.org/github/terrafrost/phpseclib/builds/740847618

With the correct certs in the test the new code passes.

By removing the whitespace and the begin/end headers, you bascially make one big B64 string, which will pass validator, when it shouldn't.

I'm just removing whitespace. Sometimes I'll copy / paste a cert and white space will get added to the end. In particular, new lines or carriage returns or both.

The issue with the certs in your example is that they had non-whitespace that was being removed in the middle of the certs.

Anyway, I updated the unit tests with this commit:

d9196e4

Thanks!

Please sign in to comment.