Skip to content

Commit 9e2aa65

Browse files
committed
Fix GH-19428: openssl_pkey_derive segfaults for DH derive with low key_length
This happens only for OpenSSL 1.1.1 because key_length is ignored for DH. It means that the provided string is overwritten with longer buffer.
1 parent cc93bbb commit 9e2aa65

File tree

3 files changed

+63
-1
lines changed

3 files changed

+63
-1
lines changed

NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ PHP NEWS
5858
. Fixed bug GH-18986 (OpenSSL backend: incorrect RAND_{load,write}_file()
5959
return value check). (nielsdos, botovq)
6060
. Fix error return check of EVP_CIPHER_CTX_ctrl(). (nielsdos)
61+
. Fixed bug GH-19428 (openssl_pkey_derive segfaults for DH derive with low
62+
key_length param). (Jakub Zelenka)
6163

6264
- PDO Pgsql:
6365
. Fixed dangling pointer access on _pdo_pgsql_trim_message helper.

ext/openssl/openssl.c

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5200,12 +5200,20 @@ PHP_FUNCTION(openssl_pkey_get_details)
52005200
}
52015201
/* }}} */
52025202

5203-
static zend_string *php_openssl_pkey_derive(EVP_PKEY *key, EVP_PKEY *peer_key, size_t key_size) {
5203+
static zend_string *php_openssl_pkey_derive(EVP_PKEY *key, EVP_PKEY *peer_key, size_t requested_key_size) {
5204+
size_t key_size = requested_key_size;
52045205
EVP_PKEY_CTX *ctx = EVP_PKEY_CTX_new(key, NULL);
52055206
if (!ctx) {
52065207
return NULL;
52075208
}
52085209

5210+
#if OPENSSL_VERSION_NUMBER < 0x30000000L
5211+
/* OpenSSL 1.1 does not respect key_size for DH, so force size discovery so it can be compared later. */
5212+
if (EVP_PKEY_base_id(key) == EVP_PKEY_DH && key_size != 0) {
5213+
key_size = 0;
5214+
}
5215+
#endif
5216+
52095217
if (EVP_PKEY_derive_init(ctx) <= 0 ||
52105218
EVP_PKEY_derive_set_peer(ctx, peer_key) <= 0 ||
52115219
(key_size == 0 && EVP_PKEY_derive(ctx, NULL, &key_size) <= 0)) {
@@ -5214,6 +5222,14 @@ static zend_string *php_openssl_pkey_derive(EVP_PKEY *key, EVP_PKEY *peer_key, s
52145222
return NULL;
52155223
}
52165224

5225+
#if OPENSSL_VERSION_NUMBER < 0x30000000L
5226+
/* Now compare the computed size for DH to mirror OpenSSL 3.0+ behavior. */
5227+
if (EVP_PKEY_base_id(key) == EVP_PKEY_DH && requested_key_size > 0 && requested_key_size < key_size) {
5228+
EVP_PKEY_CTX_free(ctx);
5229+
return NULL;
5230+
}
5231+
#endif
5232+
52175233
zend_string *result = zend_string_alloc(key_size, 0);
52185234
if (EVP_PKEY_derive(ctx, (unsigned char *)ZSTR_VAL(result), &key_size) <= 0) {
52195235
php_openssl_store_errors();

ext/openssl/tests/gh19428.phpt

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
--TEST--
2+
GH-19428: openssl_pkey_derive() DH with low key_length
3+
--EXTENSIONS--
4+
openssl
5+
--FILE--
6+
<?php
7+
8+
$priv = openssl_pkey_get_private("-----BEGIN PRIVATE KEY-----
9+
MIICJgIBADCCARcGCSqGSIb3DQEDATCCAQgCggEBAJLxRCaZ933uW+AXmabHFDDy
10+
upojBIRlbmQLJZfigDaSA1f9YOTsIv+WwVFTX/J1mtCyx9uBcz0Nt2kmVwxWuc2f
11+
VtCEMPsmLsVXX7xRUFLpyX1Y1IYGBVXQOoOvLWYQjpZgnx47Pkh1Ok1+smffztfC
12+
0DCNt4KorWrbsPcmqBejXHN79KvWFjZmXOksRiNu/Bn76RiqvofC4z8Ri3kHXQG2
13+
197JGZzzFXHadGC3xbkg8UxsNbYhVMKbm0iANfafUH7/hoS9UjAVQYtvwe7YNiW/
14+
HnyfVCrKwcc7sadd8Iphh+3lf5P1AhaQEAMytanrzq9RDXKBxuvpSJifRYasZYsC
15+
AQIEggEEAoIBAGwAYC2E81Y1U2Aox0U7u1+vBcbht/OO87tutMvc4NTLf6NLPHsW
16+
cPqBixs+3rSn4fADzAIvdLBmogjtiIZoB6qyHrllF/2xwTVGEeYaZIupQH3bMK2b
17+
6eUvnpuu4Ytksiz6VpXBBRMrIsj3frM+zUtnq8vKUr+TbjV2qyKR8l3eNDwzqz30
18+
dlbKh9kIhZafclHfRVfyp+fVSKPfgrRAcLUgAbsVjOjPeJ90xQ4DTMZ6vjiv6tHM
19+
hkSjJIcGhRtSBzVF/cT38GyCeTmiIA/dRz2d70lWrqDQCdp9ArijgnpjNKAAulSY
20+
CirnMsGZTDGmLOHg4xOZ5FEAzZI2sFNLlcw=
21+
-----END PRIVATE KEY-----
22+
");
23+
24+
$pub = openssl_pkey_get_public("-----BEGIN PUBLIC KEY-----
25+
MIICJDCCARcGCSqGSIb3DQEDATCCAQgCggEBAJLxRCaZ933uW+AXmabHFDDyupoj
26+
BIRlbmQLJZfigDaSA1f9YOTsIv+WwVFTX/J1mtCyx9uBcz0Nt2kmVwxWuc2fVtCE
27+
MPsmLsVXX7xRUFLpyX1Y1IYGBVXQOoOvLWYQjpZgnx47Pkh1Ok1+smffztfC0DCN
28+
t4KorWrbsPcmqBejXHN79KvWFjZmXOksRiNu/Bn76RiqvofC4z8Ri3kHXQG2197J
29+
GZzzFXHadGC3xbkg8UxsNbYhVMKbm0iANfafUH7/hoS9UjAVQYtvwe7YNiW/Hnyf
30+
VCrKwcc7sadd8Iphh+3lf5P1AhaQEAMytanrzq9RDXKBxuvpSJifRYasZYsCAQID
31+
ggEFAAKCAQAiCSBpxvGgsTorxAWtcAlSmzAJnJxFgSPef0g7OjhESytnc8G2QYmx
32+
ovMt5KVergcitztWh08hZQUdAYm4rI+zMlAFDdN8LWwBT/mGKSzRkWeprd8E7mvy
33+
ucqC1YXCMqmIwPySvLQUB/Dl8kgau7BLAnIJm8VP+MVrn8g9gghD0qRCgPgtEaDV
34+
vocfgnOU43rhKnIgO0cHOKtw2qybSFB8QuZrYugq4j8Bwkrzh6rdMMeyMl/ej5Aj
35+
c0wamOzuBDtXt0T9+Fx3khHaowjCc7xJZRgZCxg43SbqMWJ9lUg94I7+LTX61Gyv
36+
dtlkbGbtoDOnxeNnN93gwQZngGYZYciu
37+
-----END PUBLIC KEY-----
38+
");
39+
40+
var_dump(openssl_pkey_derive($pub, $priv, 10));
41+
42+
?>
43+
--EXPECT--
44+
bool(false)

0 commit comments

Comments
 (0)