Conversation
Automated security fix generated by OrbisAI Security
|
Analysis of PR #22524 at 3c2bb276: Test 2501 failed, but it has been 12.7% flaky lately, so it's probably NOT a fault of the PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them). Note that this CI job has had a number of other flaky tests recently (3, to be specific) so it may be that this failure is rather a systemic issue with this job and not with this specific PR. Test ../../tests/http/test_06_eyeballs.py::TestEyeballs::test_06_02_h3_only failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them). Test ../../tests/http/test_21_resolve.py::TestResolve::test_21_14_dnsd_servfail_uncached[DoH] failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them). Generated by Testclutch |
Replace the ad-hoc parenthesis/metacharacter validator in ldap_do with
REJECT_CTRL at the URL-decode layer in ldap_url_parse2_low. This rejects
percent-encoded control characters (bytes < 0x20) when the filter string
is decoded from the LDAP URL, which is the correct place to enforce this.
The previous validator incorrectly rejected valid LDAP filter syntax
characters such as '(', ')', '|', '&', '*', and '!' which are meaningful
operators per RFC 4515.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
anupamme
changed the title
ldap_do function in lib/ldap in ldap.c
bagder
left a comment
bagder
left a comment
There was a problem hiding this comment.
This seems reasonable, apart from the minor nit
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Thanks! |
3 participants
Summary
I've updated the PR. The original approach was wrong:
lud_filterholds a complete LDAP filter expression, so rejecting LDAP metacharacters like(,),|,&,*would break valid filters, and a parenthesis-balance check is not equivalent to RFC 4515 escaping.This version fixes the concrete issue at the right layer: control characters (bytes < 0x20) in per cent-encoded filter values are now rejected at URL-decode time via
REJECT_CTRL, before the string is stored inlud_filter. The previous ad-hoc validator inldap_dois removed entirely.Changes
lib/ldap.c: changeREJECT_ZERO→REJECT_CTRLinldap_url_parse2_lowwhen URL-decoding the filter component; remove the post-parse validator fromldap_doWhy this is the right place
Curl_urldecode()already supportsREJECT_CTRL(used elsewhere in the codebase), which rejects decoded bytes < 0x20. Applying it to the filter decode catches per cent-encoded control characters (%01,%0a, etc.) as they enter the filter string — before they reachldap_search_s. This is a minimal, targeted fix with a clear justification and no impact on valid LDAP filter syntax.