Skip to content

Conversation

@dengert
Copy link
Member

@dengert dengert commented Jan 18, 2024

The po->C_GetInterface is passed the callers ppInterface where *ppInterface may not be valid.

if the po->C_GetInterface may not update the *ppInterface and return an error. In this case spy_interface_function_list should not be called, as it assumes the *ppInterface has been modified.

Found debugging FireFox version 121 where FireFox passes a ppInterface where *ppInterface is not a valid pointer, causing a segfault in spy_interface_function_list.

FireFox calls C_GetInterface twice with flags = CKF_INTERFACE_FORK_SAFE twice then on third time requests with flag = 0 where po->GetInterface can support and it updates the *ppInterface with valid data.

Found with debugging #2987

On branch pkcs11-spy-segfault
Changes to be committed:
modified: pkcs11-spy.c

Checklist
  • PKCS#11 module is tested

The po->C_GetInterface is passed the callers ppInterface where
*ppInterface may not be valid.

if the po->C_GetInterface may not update the *ppInterface and return
an error. In this case  spy_interface_function_list should not be called,
as it assumes the *ppInterface has been modified.

Found debugging FireFox version 121 where FireFox passes a ppInterface
where *ppInterface is not a valid pointer, causing a segfault in
spy_interface_function_list.

FireFox calls C_GetInterface twice with flags = CKF_INTERFACE_FORK_SAFE
twice then on third time requests with flag = 0  where po->GetInterface
can support and it updates the *ppInterface  with valid data.

See OpenSC#2987

 On branch pkcs11-spy-segfault
 Changes to be committed:
	modified:   pkcs11-spy.c
Copy link
Member

@Jakuje Jakuje left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks reasonable. I think we should run the spy as part of the ci somehow too to prevent issues like this ...

@Jakuje Jakuje merged commit bc205ae into OpenSC:master Jan 23, 2024
@Jakuje Jakuje mentioned this pull request Jan 24, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants