Repository navigation
Conversation
_hx_ssl_cert_load_defaults never released its search dictionary, the search list array nor the array returned by SecItemCopyMatching, which holds the whole system root store (about 440 KB per call). Every TLS context built from the default certificates leaked them. Found with the leaks tool on a macOS host making HTTPS requests.
dimensionscape
pushed a commit
to openfl/libhxcpp
that referenced
this pull request
Oct 15, 2026
_hx_ssl_cert_load_defaults never released its search dictionary, the search list array nor the array returned by SecItemCopyMatching, which holds the whole system root store (about 440 KB per call). Every TLS context built from the default certificates leaked them. Found with the leaks tool on a macOS host making HTTPS requests. From HaxeFoundation/hxcpp#1424. Fixes #160
tobil4sk
reviewed
Oct 16, 2026
tobil4sk
left a comment
tobil4sk
left a comment
Member
There was a problem hiding this comment.
Looks reasonable, thanks! I guess after this patch there are no more leaks reported?
| CFDictionarySetValue( search, kSecMatchSearchList, CFArrayCreate(NULL, (const void **)&keychain, 1, NULL) ); | ||
| if( SecItemCopyMatching( search, (CFTypeRef *)&result ) == errSecSuccess ){ | ||
| CFDictionarySetValue( search, kSecMatchSearchList, searchList ); | ||
| result = NULL; |
Member
There was a problem hiding this comment.
Maybe this can be initialised to NULL at the declaration?
Comment on lines
+560
to
+562
| // Search for certificates. Every CF object created or copied here is | ||
| // released: this runs for every TLS connection, and the matching array | ||
| // alone holds the whole system root store (hundreds of KB). |
Member
There was a problem hiding this comment.
Suggested change
| // Search for certificates. Every CF object created or copied here is | |
| // released: this runs for every TLS connection, and the matching array | |
| // alone holds the whole system root store (hundreds of KB). | |
| // Search for certificates | |
| // Make sure to release anything we allocate |
I think it is self explanatory why we should not leak objects, so a short reminder should be enough
| if( SecItemCopyMatching( search, (CFTypeRef *)&result ) == errSecSuccess ){ | ||
| CFDictionarySetValue( search, kSecMatchSearchList, searchList ); | ||
| result = NULL; | ||
| if( SecItemCopyMatching( search, (CFTypeRef *)&result ) == errSecSuccess && result ){ |
Member
There was a problem hiding this comment.
Is there any case where it would return errSecSuccess but give a null result?
Contributor
Author
There was a problem hiding this comment.
Not sure about that, but I'd say it doesn't harm testing it
Contributor
Author
I indeed didn't find another leak after that one was patched! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
_hx_ssl_cert_load_defaults(src/hx/libs/ssl/SSL.cpp, macOS) createsor copies three Core Foundation objects it never releases:
search, the query dictionary (CFDictionaryCreateMutable);CFArrayCreate), built inline in theCFDictionarySetValuecall, so nothing even holds a reference to it;result, the array returned bySecItemCopyMatching, which holds everyroot certificate of the system keychain (about 440 KB, 1 176 items here).
Only
datandkeychainwere released. Every TLS context built from thedefault certificates therefore leaked the whole root store.
Found with the
leakstool on a macOS host making HTTPS requests throughsys.ssl.Socket: after the fix the same run reports zero leaked blocks.The patch keeps the search list in a variable, releases the three objects
after the loop, and also guards against a null
result.(Disclaimer: found this using Claude Fable 5.1)