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

Double free detected when using context-string with oqs-provider #628

Open
nakano-7107 opened this issue Jan 23, 2025 · 1 comment
Open
Labels
bug Something isn't working

Comments

@nakano-7107
Copy link

Describe the bug
I tried using the context-string feature recently added to oqs-provider. Here's the OpenSSL command I executed:

openssl dgst -provider default -provider oqsprovider -sign key.pem \
              -sigopt context-string:1122 -out sign.bin msg.bin  

However, the command resulted in the following error and abnormal termination:

double free detected

Is this the correct way to use the feature?

To Reproduce
Steps to reproduce the behavior:

  1. Use oqs-provider with a key file (key.pem) and a message file (msg.bin).
  2. Run the above OpenSSL command.
  3. Observe the "double free detected" error.

Suggested Fix
I modified the code as follows, and the "double free detected" issue no longer occurred:

diff --git a/oqsprov/oqs_sig.c b/oqsprov/oqs_sig.c  
index b6f07a70..000c7dfb 100644  
--- a/oqsprov/oqs_sig.c  
+++ b/oqsprov/oqs_sig.c  
@@ -1213,8 +1213,8 @@ static void oqs_sig_freectx(void *vpoqs_sigctx) {  
     OPENSSL_free(ctx->aid);  
     ctx->aid = NULL;  
     ctx->aid_len = 0;  
-    OPENSSL_free(ctx->context_string);  
-    ctx->context_string = NULL;  
+//    OPENSSL_free(ctx->context_string);  
+//    ctx->context_string = NULL;  
     ctx->context_string_length = 0;  
     OPENSSL_free(ctx);  
 }  

This fix comments out the deallocation of ctx->context_string.

Environment

  • OS: Ubuntu 22.04.1
  • OpenSSL version 3.4.0
  • oqsprovider version 0.8.0
@nakano-7107 nakano-7107 added the bug Something isn't working label Jan 23, 2025
@baentsch
Copy link
Member

Thanks for the report, @nakano-7107 !

Is this the correct way to use the feature?

It seems so -- but I've got to admit we don't have a test case for that, so you may very well have hit on a problem.

Would you be willing to add test case and fix proposal via PR?

However, I'm not entirely convinced that your change is the correct fix; I have the hunch that the allocation in OSSL_PARAM_get_octet_string is the probable culprit (with openssl core managing the memory and not oqsprovider. But let's see when we add the test case, e.g., to any of the command line tests triggered by "scripts/runtests.sh").

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
None yet
Development

No branches or pull requests

2 participants