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.

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

Null Pointer Dereference in Crypto_Check_Anti_Replay() #177

Closed
spicydll opened this issue Jul 2, 2023 · 1 comment
Closed

Null Pointer Dereference in Crypto_Check_Anti_Replay() #177

spicydll opened this issue Jul 2, 2023 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@spicydll
Copy link

spicydll commented Jul 2, 2023

I noticed some code that could allow for a null pointer to be dereferenced in src/src_main/crypto.c

int32_t Crypto_Check_Anti_Replay(SecurityAssociation_t *sa_ptr, uint8_t *arsn, uint8_t *iv)
{
    int32_t status = CRYPTO_LIB_SUCCESS;
    int8_t IV_VALID = -1;
    int8_t ARSN_VALID = -1;

    // Check for NULL pointers
    if (arsn == NULL && sa_ptr->arsn_len > 0)
    {
        return CRYPTO_LIB_ERR_NULL_ARSN;
    }
    if (iv == NULL && sa_ptr->shivf_len > 0 && crypto_config->cryptography_type != CRYPTOGRAPHY_TYPE_KMCCRYPTO)
    {
        return CRYPTO_LIB_ERR_NULL_IV;
    }
    if (sa_ptr == NULL)
    {
        return CRYPTO_LIB_ERR_NULL_SA;
    }
    // rest of Crypto_Check_Ani_Replay() ...

Notice how sa_ptr is derefenced in both of the first two if statements (sa_ptr->arsn_len and sa_ptr->shivf_len). The sa_ptr is checked only after these first two dereferences. Therefore, a null pointer dereference can occur which can cause a crash.

These if statements should be reordered as follows to fix the issue:

int32_t Crypto_Check_Anti_Replay(SecurityAssociation_t *sa_ptr, uint8_t *arsn, uint8_t *iv)
{
    int32_t status = CRYPTO_LIB_SUCCESS;
    int8_t IV_VALID = -1;
    int8_t ARSN_VALID = -1;

    // Check for NULL pointers
    if (sa_ptr == NULL)
    {
        return CRYPTO_LIB_ERR_NULL_SA;
    }
    if (arsn == NULL && sa_ptr->arsn_len > 0)
    {
        return CRYPTO_LIB_ERR_NULL_ARSN;
    }
    if (iv == NULL && sa_ptr->shivf_len > 0 && crypto_config->cryptography_type != CRYPTOGRAPHY_TYPE_KMCCRYPTO)
    {
        return CRYPTO_LIB_ERR_NULL_IV;
    }
    // rest of Crypto_Check_Ani_Replay() ...
spicydll added a commit to spicydll/CryptoLib that referenced this issue Jul 2, 2023
@jlucas9 jlucas9 self-assigned this Jul 6, 2023
@jlucas9 jlucas9 added the bug Something isn't working label Jul 6, 2023
@rjbrown2
Copy link
Contributor

This modification is valid.

rjbrown2 pushed a commit that referenced this issue Oct 13, 2023
@rjbrown2 rjbrown2 mentioned this issue Oct 13, 2023
jlucas9 added a commit that referenced this issue Oct 19, 2023
…rypto_check_anti_replay

#177 Fix - per SpicyDLL - solves possibility of sa NULL dereference
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
3 participants