libcurl with Secure Transport and --enable-optimize crashes when settings CURLOPT_SSLCERT to an inexistant cert #9194
Comments
|
@nickzman can you see if you can make anything from this? |
|
Copy that; I'll see if I can reproduce this. |
|
I can't reproduce the problem with the example provided; it properly exits without finding the certificate. I'm scanning the code, and nothing really looks like it would obviously cause the problem. Can you still reproduce the problem with the latest code? |
|
Thanks for taking a look @nickzman ! Here's a more "plug and play" sample which reproduces the crash on for me. You should only have to run (So to answer your question, I can reproduce using libcurl 7.84.0 and nghttp2 1.48.0) Here's a video of me running it on my machine : Enregistrement.de.l.ecran.2022-07-25.a.17.41.44.mov |
|
I ran the script, but still could not reproduce the problem on my M1 Mac running the latest macOS release. What version of Clang are you using? I'm using Apple Clang 13.1.6, which comes with Xcode 13.4.1. |
Also here's the full log of my |
|
Ok so I have 2 coworkers (Intel machines) who are crashing like me, and a coworker (Apple Silicon) not crashing and getting the error message like you are. I'll compare our configs / build logs and report back. |
|
Some news : I could not find any meaningful differences between my coworker's build logs and mine. After a bit more debugging in lldb, it appears the bug is caused by Apparently in that case That doesn't looks too good to me. However that doesn't explain why it works on some machines on not others (mine 😭). It's starting to look more and more like an Apple bug, though, so I'm not sure if I should continue to bother you guys about it. If you have any pointer on how I could possibly debug this further I'd appreciate it, but either way thank you already for the time you have spent on this issue 👍🏻 I'll probably file a radar if I can reduce the scope of the issue further (not having Apple engineers build nghttp2 and curl to reproduce would be nice), but I think I already know how this will end... |
|
Could this be worked around in curl by checking if |
|
Does this still happen with a current curl? |
|
@bagder yes, I just reproduced the crash with the latest version of curl (8.2.1) and nghttp2 (1.55.1) targeting macOS 13. Here's and updated version of my repro script : repro-curl-9194-20230728.zip |
|
This actually seems to be a security vulnerability (denial of service) in either libcurl or Secure Transport. |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
Does this simple patch fix it? diff --git a/lib/vtls/sectransp.c b/lib/vtls/sectransp.c
index 348bbe202..1c72ed626 100644
--- a/lib/vtls/sectransp.c
+++ b/lib/vtls/sectransp.c
@@ -1147,11 +1147,12 @@ static OSStatus CopyIdentityWithLabel(char *label,
CFRetain(identity);
*out_cert_and_key = identity;
status = noErr;
break;
}
- CFRelease(common_name);
+ if(common_name)
+ CFRelease(common_name);
}
CFRelease(cert);
}
}
|
When SecCertificateCopyCommonName() returns NULL, the common_name pointer remains set to NULL which apparently when calling CFRelease() on (sometimes?) crashes. Reported-by: Guillaume Algis Fixes #9194
|
@bagder with the patch it's better in some configurations, but I'm still crashing with
When configuring with But because I don't have the debug symbols in this case, I'm struggling to understand which part of the code causes the segfault. My lldb debugging skills are a bit rusty ; a |
|||||||||||||||||||||||||
|
Frame 2 says it is now inside |
|
@bagder after digging, it seems like the lldb backtrace was incomplete, and the problem is still at the same place (on the I think the problem comes from the fact that when testing whether I'm guessing that when curl is compiled with debug flags the compiler will not optimize things (?) and will take the time to initialize So the crash is fixed with : diff --git a/lib/vtls/sectransp.c b/lib/vtls/sectransp.c
index 32bb3a5..0391baf 100644
--- a/lib/vtls/sectransp.c
+++ b/lib/vtls/sectransp.c
@@ -1086,7 +1086,7 @@ static OSStatus CopyIdentityWithLabel(char *label,
CFArrayRef keys_list;
CFIndex keys_list_count;
CFIndex i;
- CFStringRef common_name;
+ CFStringRef common_name = NULL;
/* SecItemCopyMatching() was introduced in iOS and Snow Leopard.
kSecClassIdentity was introduced in Lion. If both exist, let's use them
@@ -1149,7 +1149,8 @@ static OSStatus CopyIdentityWithLabel(char *label,
status = noErr;
break;
}
- CFRelease(common_name);
+ if(common_name)
+ CFRelease(common_name);
}
CFRelease(cert);
}
|
|
I have updated the PR with (a slight variation of) this. |
When SecCertificateCopyCommonName() returns NULL, the common_name pointer remains set to NULL which apparently when calling CFRelease() on (sometimes?) crashes. Reported-by: Guillaume Algis Fixes curl#9194 Closes curl#11554
I did this
--enable-optimizeduring configuration (this bellow for the full options list).curl_easy_setopt(handle, CURLOPT_SSLCERT, "thisdoesnotexistinkeychain");).I expected the following
curl_easy_performreturns an error.What I got
curl_easy_performcrashes on aCFReleasecall.curl/libcurl version
Operating system
Details
libcurl ./configure options
Minimal sample reproducing the issue
The program will crash on :
Apparently curl is releasing the
keys_listpointer even if nothing has been found bySecItemCopyMatching, which causes the crash. I spent a good few hours on this trying to figure out whykeys_listwas not NULL, but apparently my C is too rusty to solve this :(Any help would be greatly appreciated !
The text was updated successfully, but these errors were encountered: