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

Missing sc_free() or similar #2054

Closed
rliebscher opened this issue Jun 10, 2020 · 3 comments · Fixed by #2062
Closed

Missing sc_free() or similar #2054

rliebscher opened this issue Jun 10, 2020 · 3 comments · Fixed by #2062

Comments

@rliebscher
Copy link
Contributor

Problem Description

When using the function sc_asn1_sig_value_rs_to_sequence() on Windows using the DLL, I cannot free the returned buffer.

The reason is that the provide OpenSC DLL use a different C runtime then the program loading the DLL. So calling a free in my program might use a different heap implementation then the DLL has compiled in, which leads to problems. (Might even crash).

Actually to the OpenSC 32bit DLL references only Windows system DLLs, so there is no chance to get the correct free() implementation as all is internal to the OpenSC DLL.

You have a similar problem if you load the DLL using cffi, then you need a C runtime function to free the buffer. (this is not only Windows)

For other data structure there exists corresponding functions (sc_file_new and sc_file_free, and others). But there is no function to return the buffer I get with sc_asn1_sig_value_rs_to_sequence.

Proposed Resolution

Add to sc.c a new function and export it in the DLL interface:

void sc_free(void *p) { free(p); }

So I can free any buffer opensc has allocated by using a dedicated opensc function, which calls the correct free implementation.

There might need to be added also some hints at function documention of sc_asn1_sig_value_rs_to_sequence (and others ?) to use sc_free to free the returned buffer.

Steps to reproduce

Create a small test program using the function above and compile it with MinGW (http://http://mingw-w64.org). MinGW will link against MSVCRT and use its free function.
If run in debugger it stops at the free with "Thread 1 received signal SIGTRAP, Trace/breakpoint trap."
Without debugger you get a message box like this "Access violation at address 76FD68EA in module 'ntdll.dll'. Read of address C2D71BFD."

Logs

@Jakuje
Copy link
Member

Jakuje commented Jun 10, 2020

I don't think we are claiming libopensc to be a public API. It is changing all the time and it is considered internal for the tools and libraries built together with OpenSC.

If you are fine with this, the easiest solution would be to provide a PR with your suggested changes. We do not have a reason to reject something that will improve (even internal) documentation nor this API if it will make things working for you.

@dengert
Copy link
Member

dengert commented Jun 10, 2020

libopensc.exports already has these:

scconf_free
sc_aux_data_free
sc_file_free
sc_free_apps
sc_free_ef_atr
sc_mem_secure_free
sc_pkcs15_card_free
sc_pkcs15_free_tokeninfo
sc_pkcs15_free_cert_info
sc_pkcs15_free_certificate
sc_pkcs15_free_data_info
sc_pkcs15_free_data_object
sc_pkcs15_free_key_params
sc_pkcs15_free_object
sc_pkcs15_free_auth_info
sc_pkcs15_free_prkey
sc_pkcs15_free_prkey_info
sc_pkcs15_free_pubkey
sc_pkcs15_free_pubkey_info
sc_pkcs15_free_tokeninfo

and ./tools/pkcs15-crypt.c and ./tools/pkcs11-tool.c both call sc_asn1_sig_value_rs_to_sequence.
I never tried running pkcs11-tool with "sequence" on windows. So it may fail too as it looks like it does a free(seq);

It would not take much to add a sc_asn1_sig_value_rs_to_sequence_free.
There are a lot of other exported routines, that might have the same problem.
I see pkcs11-tool.c calls sc_asn1_encode_object_id. Any of the sc_asn1_encode* return a
buffer the is allocated (and realloc'ed), which return der encoded buffer, that could be freed with a
sc_asn1_encode_free.

Its not clear if pkcs11-tool.c cleans up as it should. I see when running it under
valgrind there are a number of left over blocks.

sc_asn1_sig_value_rs_to_sequence could also be renamed to sc_asn1_encode_rs as it is really just another sc_asn1_encode routine.

@rliebscher
Copy link
Contributor Author

All these functions mentioned above have some special data type as argument. But for freeing a generic buffer there is no function now.

I also see no real sense to add a sc_asn1_sig_value_rs_to_sequence_free() and maybe more special free functions for other functions which just free a plain unstructured buffer. A common sc_free(void *) would do it.

Also all tools build at the same time as the Windows DLL will not have a problem with free, because they use the same free as the DLL internally. So your Windows delivery has no problem with it.

So the problem occurs only when you start use the DLL in another program, which might have another C runtime implementation. (On Linux this is never a problem as all use the system C library)

rliebscher added a commit to rliebscher/OpenSC that referenced this issue Jun 16, 2020
For more details see OpenSC#2054
Jakuje pushed a commit that referenced this issue Jun 22, 2020
For more details see #2054
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 a pull request may close this issue.

3 participants