Fix NULL pointer crash in OQS KEM encaps/decaps error handling #184
+11
−7
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change looks to address a crash reported by Roumen Petrov (pkixssh@roumenpetrov.info), the developer of PKIX-SSH. The vulnerability exists in the error handling paths of
kexoqsecdh.candkexoqsx25519.cin theOQS_KEM_encaps()orOQS_KEM_decaps()functions. When OQS key encapsulation operations fail, the return value does not indicate a failure. On the error path the code uses the value0for variablerset from previous operations, causing the code to try to use either server_blobp or shared_secretp causing a NULL => bus error and crash.In
kexoqs.c, error handing is handled before jumping to the cleanup code in theOQS_KEM_encapsandOQS_KEM_decapsfunctions by settingr = SSH_ERR_LIBCRYPTO_ERROR;. This PR makes a change to have the error handling logic behave the same way inkexoqsecdh.candkexoqsx25519.c.I was able to reproduce this crash using the PKIX-SSH client and the OQS-SSH server when KEX algorithm list truncation caused a mismatch and failure by triggering a MAX_PROP buffer limitation in the client's
match.cfile. The confirmation was seeingmm_reap: preauth child terminated by signal 11in the server logs, and then verifying that line was no longer present after this change.The reproduction steps/attack scenario this addresses are as follows:
OQS_KEM_encaps()orOQS_KEM_decaps()fails to process the bad datar=0despite the failure*server_blobp,*shared_secretp) remain NULL