Possible Null Pointer Dereference in `TLSWrap::PskClientCallback`

[wooffie]

Version

20.18.0

Platform

Subsystem

crypto

What steps will reproduce the bug?

Problem around with part of code - https://github.com/nodejs/node/blob/da5f7aca6ac1fac2b7840dc11c0ef8e740cfc414/src/crypto/crypto_tls.cc#L1559C1-L1564C58

After creating Utf8Value object code checks its length, but not checks for nullptr. After this nullptr can be dereferenced in memcpy call

How often does it reproduce? Is there a required condition?

Condition - identity_buf stores nullptr

What is the expected behavior? Why is that the expected behavior?

Return 0, for consistent API, for example

if (*identity_buf == nullptr || identity_buf.length() > max_identity_len)
    return 0;

What do you see instead?

Additional information

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Reporter: Burkov Egor (eburkov@rvision.ru).

Organization: R-Vision (support@rvision.ru).

[tniessen]

After creating Utf8Value object code checks its length, but not checks for nullptr. After this nullptr can be dereferenced in memcpy call

How often does it reproduce? Is there a required condition?

Condition - identity_buf stores nullptr

@wooffie Could you please clarify how this could happen and then lead to dereferencing a nullptr? As far as I can tell, only an empty Utf8Value could contain a nullptr (because allocations are checked), in which case its length would be zero and the pointer *identity_buf would never be dereferenced. (And even then, I am not sure it would contain a nullptr because small sizes should rely on stack allocation.)

[github-actions]

This issue/PR was marked as stalled, it will be automatically closed in 30 days. If it should remain open, please leave a comment explaining why it should remain open.

[github-actions]

Closing this because it has stalled. Feel free to reopen if this issue/PR is still relevant, or to ping the collaborator who labelled it stalled if you have any questions.