node icon indicating copy to clipboard operation
node copied to clipboard

Possible Null Pointer Dereference in `TLSWrap::PskClientCallback`

Open wooffie opened this issue 1 year ago • 2 comments

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 ([email protected]).

Organization: R-Vision ([email protected]).

wooffie avatar Jan 20 '25 12:01 wooffie

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.)

tniessen avatar Mar 18 '25 19:03 tniessen

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[bot] avatar Jun 15 '25 14:06 github-actions[bot]

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.

github-actions[bot] avatar Jul 16 '25 00:07 github-actions[bot]