Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Null Ptr Deref in CMS_decrypt_set1_pkey_and_peer #19975

Closed
mboehme opened this issue Dec 29, 2022 · 2 comments
Closed

Null Ptr Deref in CMS_decrypt_set1_pkey_and_peer #19975

mboehme opened this issue Dec 29, 2022 · 2 comments
Labels
branch: master Merge to master branch severity: important Important bugs affecting a released version triaged: bug The issue/pr is/fixes a bug

Comments

@mboehme
Copy link

mboehme commented Dec 29, 2022 •
edited by DDvO
Loading

We found a null pointer deref during the decryption of a CMS encrypted message that seems to exist only in master.

When the private key is modified (see below), the call to ossl_cms_get0_env_enc_content in CMS_decrypt_set1_pkey_and_peer returns a null pointer (crypto/cms/cms_smime.c#L710). This null pointer is dereferenced and used in the subsequent call to OPENSSL_clear_free. We did not investigate whether there are other circumstances under which ossl_cms_get0_env_enc_content (crypto/cms/cms_env.c:141) may return null. The bug was introduced about a month ago with a fix of a memory leak (25dd780).

How to reproduce:

cat << EOL | base64 -d > encrypted.cms
TUlNRS1WZXJzaW9uOiAxLjAKQ29udGVudC1EaXNwb3NpdGlvbjogYXR0YWNobWVudDsgZmlsZW5h
bWU9InNtaW9lLnA3bSIKQ29udGVudC1UeXBlOiBhcHBsaWNhdGlvbi9wa2NzNy1taW1lOyBzbWlt
ZS10eXBlPWUBdmVsb3BlZC1kYXQ7IG5hbW0iCkNvbnRlbnQtVHJhbnNmZXItRW5jb2Rpbmc6gAAA
AGU2NAoKTUlBR0NTcUdTSVUzRFFFSEE2Q0FNSUFDQVFBeGdnRmdNSUlCWEFJQkFEQkVNQzB4S3pB
cEJnTlZCQU1USWxOaApiWEJzWlNCTVFVMVFVeUJEWlhKMGFXWnBZMkYwWlNCQmRYUm9iM0pwZEhr
Q0U2RTFlR2RJcml0UEwyVDl0S0U0CmYvNFF2QVF3RFFZSktvWklodmNOQVFFQkJRQUVnZ0VBSXRl
NEFHL1dDb21aTEpvb1BLTEhWUEZHVDBjZXhMQmwKeGhBSWdBZFZVMDBWMDFaMmt6cXBuUjJjL2Nq
OWlRL3E1WjNIZHMrOXdHYmE2SENhS3FPOTZ3N0x2RURWaFo3QQpvNnk1K0IvaytIRmtqQm9kZitG
QTd3czRYRFUwM3pTS2ZCSHV2dFRrcXdtL0dKdWNIbEpJd05sblBVcktKZjRNCjNBZVkwR1hpOXp6
NmIwSHl4OHZQQ0MreTA5bWcxR3Zxbk1zWU1uOWJHQ3ErSk1HeXdxdGJuSkhtRmluZGFuY1MKMDJs
dmlkRzVMNER4Rnl5c2FaT3ZjVHVyaEhkWFF4c3NnWExuVkJtNS9iZFdDVW15SGpyQkx1dmZ5eUUv
K0xycwp2WjkzZ2REcjJZdExacFRUMm5Uc0xuVUhMejU5d2dDSG5ZWVNCYTRmUW52bzhBcW5lcUdM
SGpDQUJna3Foa2lHCjl3MEJCd0V3RkFZSUtvWklodmNOQXdjRUNDcVhyYURXbElveG9JQUVVTWhm
UWNhaXRmSlg5WlhiQjN1c0l0dlQKMXU5RzFSa3J0QnNZT3IxUS80MWJ6czFtNVV6bUNtV1BQMnNp
SW5ra0xWS0ViWW53TFQwMmU5b21GTjdsMlhSMgpOTitsQXhPckVSNUZmcE1zN25JekJBaFJpZzJ6
TTJtU3NnQUFBQUFBQUFBQUFBQT0KCg==
EOL

cat << EOL | base64 -d > private.key
MIIE+gIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQC09InoWDgWPk2af0+StijSNOR8
K/hN8D+l078oullsk4ASvSwjsCNo7sHUa4xQUl5JO6VqY18LANwORjrc9BaX4MguzsbFXBe6uFh1
mVpXmFxSpUByQ+950MFz/evPgP96wV+z4TtAwW2Z34rTiz4DxMI07XYNFUEOls/gkUP2GxzymsO2
kaYWTut3SryCqeHEFbZFkB4urMk4xrIJC3CzWruS2Q0FHbBlfkgKN5wXVgkWFfiOucfCn+IQsaqp
o1d3f9jSkbtAV5w3vzfog8919MxKI9H6l4KuElnAtJ7BtZcsl7dUy9u9COgEykRiVokFQgqQ7XND
U+r3SeOWwks7AgMBAAECggEAFKD2DG9A1u77q3u3p2WDH3zueTtiqgaT8u8XO+jhOI/+HzoX9eo8
DIJ/b/G3brwHyfh17JFvLH1zbgsn5bghJTz3r+JcZZ5l3srqMV8t8zjIJEHOKC3szH8gYVKWrIgB
AqOt1H9Ti8J2oKk2aymqBFr3ZXpBUCTWpEz2s3FMBUUIqCEsAJqsdEch+kt43X5kvAom7LC1DHiE
6RKfhMEub/LGNHSwY4dmzhaG6p95FJ1hs8HoURI2ReVpsTadaKd3KoYNc1lcffmwdZs/hFs7xmmw
XKMmlonh1mzHqD1/BqeJHc8MP4ueDdyVgIe/uVtlQ9NcRQbuokkDyDYMYV6hzQKBgQD75ahYGFGZ
znRKtSE3w/2rUqTYIWxx2PQz5G58PcsTZM89Hj4aZOoLmudHbrTQHluRNcHoXEI62rs0cVPsD7Il
ZOLfs+SSTeNEXxD57mjyyufpV65OcNc1mSJAmMX2jWQ8ndnOuWPcc5J6fNvTau0a7ZBOaeKHnA8X
XL3GYilM9QKBgQC35xKi7f2JmGtsYY21tfRuDUm6EjhMW6b7GWnI9IXF8TGj15s7oDEYvqSPTJdB
6PAb/tZwdbj9mB4qj176x1kB/N7GO974O8UP/PdHkU7duyf5nRq1mrI+yGFHVsGD313rc+akYdKc
C207e6IRMST1ZFoznC6qNgpinNTuDz4ZbwKBgA5Dd9/dKKm77gvY69Objn6oBFuUsO5VaaaSlcsF
OL2VZMLCNqQJ+NLFZ7k8xJJQVcEIOT2uE7X/csBKdoUUcnL5nnsqVZQPQwI5G937KQgugylMZLte
WmFXlX/w5qzKXtWr3ox9JPFzveSfs1bqZBi1QQmfp0skhBo/jyNvpYUNAoGAMNkwGhcdQW87GY7Q
FXQ/ePwOmV49lgrCT/BwKPDKl8l5ZgvfL/ddEzWQgH/XraoyHT2TuEuM18+QM73hfLt26RBCHGXK
1CUMMzL+fAQc7sjH1YXlkleFASg4rrpcrKqoR+KBYSiayNhAK4yrf+WN66C8VPknbA7us0L1TEbA
OAECgYEAtwRiiQwk3BlqENFypyc80Q1pxp3U7ciHi8mni0kNcTqe57Y/2o8nY9ISnt1GffMs79YQ
fRXTRdEm2St6oChI9Cv5j74LHZXkgEVFfO2Nq/uwSzTZkePk+HoPJo4WtAdokZgRAyyHl0gEae8R
l89eyBX7dutONALjRZFTrg18CuegOzA5BgorBgEEAZIIEggBMSswKQYJYIZIAWUDBAICBBySyJ1D
MNPY4x1P3pudD+bp/BQhQd1lpF5bQ28F
EOL

./apps/openssl cms -decrypt -in encrypted.cms -stream -inkey private.key

This shows

AddressSanitizer:DEADLYSIGNAL
=================================================================
==48551==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000028 (pc 0x56056dfacd24 bp 0x7ffe30008c60 sp 0x7ffe30008c00 T0)
==48551==The signal is caused by a READ memory access.
==48551==Hint: address points to the zero page.
    #0 0x56056dfacd23 in CMS_decrypt_set1_pkey_and_peer crypto/cms/cms_smime.c:713
    #1 0x56056dae1008 in cms_main apps/cms.c:1165
    #2 0x56056db2b282 in do_cmd apps/openssl.c:418
    #3 0x56056da91c25 in main apps/openssl.c:298
    #4 0x7f7df710f082 in __libc_start_main ../csu/libc-start.c:308
    #5 0x56056daa74fd in _start (./apps/openssl+0x2bd4fd)

The fix should be straight forward. Similar mem-leaks were merged as part of #19222 and may need similar checks.

Found during fuzzing.
@mboehme mboehme added the issue: bug report The issue was opened to report a bug label Dec 29, 2022
@t8m t8m added branch: master Merge to master branch triaged: bug The issue/pr is/fixes a bug severity: important Important bugs affecting a released version and removed issue: bug report The issue was opened to report a bug labels Jan 2, 2023
@t8m
Copy link
Member

t8m commented Jan 2, 2023

@DDvO might want to take a look

DDvO added a commit to siemens/openssl that referenced this issue Jan 2, 2023
@DDvO
CMS_decrypt_set1_*(): fix NULL deref on unsuitable content type
865d50d 
Fixes openssl#19975
DDvO added a commit to siemens/openssl that referenced this issue Jan 2, 2023
@DDvO
CMS_decrypt_set1_*(): fix NULL deref on unsuitable content type
e126b85 
Fixes openssl#19975
for CMS_decrypt_set1_pkey_and_peer() in the obvious way,
and a related potential crash in CMS_decrypt_set1_password().

The point is that the input might have an unexpected content type,
so a guard is needed at both places after `ec` is obtained.

Note that in CMS_decrypt_set1_pkey_and_peer() there was
no such ec != NULL guard for
```
    if (ris != NULL)
        debug = ec->debug;
```
maybe because it is implied here by ris != NULL.
@DDvO DDvO mentioned this issue Jan 2, 2023
Closed
DDvO added a commit to siemens/openssl that referenced this issue Jan 2, 2023
@DDvO
CMS_decrypt_set1_*(): fix NULL deref on unsuitable content type
109fcf2 
Fixes openssl#19975
for CMS_decrypt_set1_pkey_and_peer() in the obvious way,
and a related potential crash in CMS_decrypt_set1_password().

The point is that the input might have an unexpected content type,
so a guard is needed at both places after `ec` is obtained.

Note that in CMS_decrypt_set1_pkey_and_peer() there was
no such ec != NULL guard for
```
    if (ris != NULL)
        debug = ec->debug;
```
maybe because it is implied here by ris != NULL.
@DDvO
Copy link
Contributor

DDvO commented Jan 2, 2023

Thank you @mboehme for reporting this.
Fix is in #19981.

DDvO added a commit to siemens/openssl that referenced this issue Feb 3, 2023
@DDvO
CMS_decrypt_set1_*(): fix NULL deref on unsuitable content type
21d0f77 
Fixes openssl#19975
for CMS_decrypt_set1_pkey_and_peer() in the obvious way,
and a related potential crash in CMS_decrypt_set1_password().

The point is that the input might have an unexpected content type,
so a guard is needed at both places after `ec` is obtained.

Note that in CMS_decrypt_set1_pkey_and_peer() there was
no such ec != NULL guard for
```
    if (ris != NULL)
        debug = ec->debug;
```
maybe because it is implied here by ris != NULL.
openssl-machine pushed a commit that referenced this issue Feb 24, 2023
@DDvO
CMS_decrypt_set1_*(): fix NULL deref on unsuitable content type
ceb767b 
Fixes #19975
for CMS_decrypt_set1_pkey_and_peer() in the obvious way,
and a related potential crash in CMS_decrypt_set1_password().

The point is that the input might have an unexpected content type,
so a guard is needed at both places after `ec` is obtained.

Note that in CMS_decrypt_set1_pkey_and_peer() there was
no such ec != NULL guard for
```
    if (ris != NULL)
        debug = ec->debug;
```
maybe because it is implied here by ris != NULL.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
(Merged from #20209)
openssl-machine pushed a commit that referenced this issue Feb 24, 2023
@DDvO
CMS_decrypt_set1_*(): fix NULL deref on unsuitable content type
da30fd8 
Fixes #19975
for CMS_decrypt_set1_pkey_and_peer() in the obvious way,
and a related potential crash in CMS_decrypt_set1_password().

The point is that the input might have an unexpected content type,
so a guard is needed at both places after `ec` is obtained.

Note that in CMS_decrypt_set1_pkey_and_peer() there was
no such ec != NULL guard for
```
    if (ris != NULL)
        debug = ec->debug;
```
maybe because it is implied here by ris != NULL.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
(Merged from #20209)

(cherry picked from commit ceb767b)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
branch: master Merge to master branch severity: important Important bugs affecting a released version triaged: bug The issue/pr is/fixes a bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

CMS_decrypt_set1_*(): fix NULL deref on unsuitable content type siemens/openssl
3 participants
@DDvO @mboehme @t8m