Added ChallengeCard annotation to LDAPInjectionVulnerability.java and corresponding message keys to messages.properties - #672
Added ChallengeCard annotation to LDAPInjectionVulnerability.java and corresponding message keys to messages.properties#672An16og wants to merge 5 commits into
Conversation
… corresponding message keys to messages.properties
… corresponding message keys to messages.properties
📝 WalkthroughWalkthrough
ChangesLDAP Injection Challenge Mode
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #672 +/- ##
=========================================
Coverage 54.46% 54.46%
Complexity 663 663
=========================================
Files 91 91
Lines 3604 3604
Branches 399 399
=========================================
Hits 1963 1963
Misses 1463 1463
Partials 178 178 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| LDAP_INJECTION_LEVEL_4_HINT_2=This transformation maps control sequences cleanly into safe hexadecimal representations, meaning parentheses logic cannot be broken out of. | ||
| LDAP_INJECTION_LEVEL_4_HINT_3=Observe if simple wildcard characters still alter execution parameters or if they are evaluated entirely as safe search strings. | ||
| LDAP_INJECTION_LEVEL_4_PAYLOAD_DESCRIPTION=Attempted injection vector proving defensive validation metrics via value encoding handlers. | ||
| LDAP_INJECTION_LEVEL_4_PAYLOAD_VALUE=* |
There was a problem hiding this comment.
I was not able to exploit this level with this payload. can you please fix this?
There was a problem hiding this comment.
I think we should mark this level as secure. can you please fix it?
| LDAP_INJECTION_LEVEL_3_HINT_2=If you inject structural parentheses, can you shift the query evaluation boundaries so it validates a target user? | ||
| LDAP_INJECTION_LEVEL_3_HINT_3=Inject a value like 'admin)(uid=*' to cleanly balance the server's tracking filters. | ||
| LDAP_INJECTION_LEVEL_3_PAYLOAD_DESCRIPTION=Authentication query injection leveraging attribute filtering patterns. | ||
| LDAP_INJECTION_LEVEL_3_PAYLOAD_VALUE=admin)(uid=* |
There was a problem hiding this comment.
this is not the right payload. can you please look at hints and suggest again?
There was a problem hiding this comment.
this is not the right payload. can you please look at hints and suggest again?
Looking at the code, even if we inject an LDAP breakout in the username parameter, the Java code still loops through the results and runs verifyPassword(password, storedPassword).
Can you please clarify the intended exploit payload for this level? Is it meant to be a simple wildcard * for account harvesting/password spraying, or is there a specific injection string that completely skips the password loop based on the test data? Want to make sure my hints and payload values match the exact design of the level!
There was a problem hiding this comment.
Can you look at attack vector annotation and see what it says?
|
|
||
| # LDAPInjectionVulnerability Challenge Mode | ||
|
|
||
| LDAP_INJECTION_LEVEL_1_CHALLENGE=Manipulate the simple LDAP filter query to return all directory entries. |
There was a problem hiding this comment.
I think here we can have 2 challenges, 2nd challenge is to retrieve entire database using queries. we can ask user to construct query to fetch entire database
| LDAP_INJECTION_LEVEL_1_PAYLOAD_DESCRIPTION=Wildcard injection payload to list all matching records inside the directory scope. | ||
| LDAP_INJECTION_LEVEL_1_PAYLOAD_VALUE=* | ||
|
|
||
| LDAP_INJECTION_LEVEL_2_CHALLENGE=Bypass the structural OR logical grouping statement to dump the entire user directory. |
There was a problem hiding this comment.
I think we should ask user to find out the email for each of the users. May be make it challenge 2 and help find out the emails for the user
| LDAP_INJECTION_LEVEL_5_HINT_2=You can inject boolean logic extensions into the username parameter to test for specific true/false responses. | ||
| LDAP_INJECTION_LEVEL_5_HINT_3=Test for structural characters iteratively by combining wildcards with prefix strings to determine specific user profile properties. | ||
| LDAP_INJECTION_LEVEL_5_PAYLOAD_DESCRIPTION=Boolean blind directory evaluation payload testing for structural query changes. | ||
| LDAP_INJECTION_LEVEL_5_PAYLOAD_VALUE=admin*)(userPassword=* No newline at end of file |
There was a problem hiding this comment.
This is invalid payload. i would suggest trying out the levels with the payload. please fix the challenge as well as hints and payloads
|
@An16og is this closed by mistake? let me know if you need help. |
I did that by mistake. I can definately use some help regarding the change you suggest in level 1 (Adding 2 challenge card instead of 1) how are we planning to do that. |
Just add challengecard annotation twice that's it. |
… corresponding message keys to messages.properties
…nto LDAPInjection # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
Adding ChallengeMode to LDAPInjectionVulnerability and adding corresponding keys to message.properties
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/org/sasanlabs/service/vulnerability/ldapInjection/LDAPInjectionVulnerability.java`:
- Around line 273-274: The description key LDAP_LEVEL_4_SECURE used in the
LDAPInjectionVulnerability class is missing from the i18n messages.properties
file. Add the missing internationalization key LDAP_LEVEL_4_SECURE to the
messages.properties file with an appropriate value for the Level 4 secure LDAP
injection description. This will ensure the label is properly resolved when
consumed by metadata consumers and prevent fallback behavior.
In `@src/main/resources/i18n/messages.properties`:
- Around line 628-630: The LDAP_INJECTION_LEVEL_5_HINT_3 property and
LDAP_INJECTION_LEVEL_5_PAYLOAD_VALUE property contain mismatched payloads. The
hint shows I*)(uid=*, * while the payload value shows *)(uid=*, * (missing the
leading I character). Align these two properties by ensuring both contain the
same payload value so users receive consistent guidance. Verify which version is
the correct payload and update whichever property is incorrect to match.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 88ea3c3f-9081-4d8a-bb1b-074b5a58bb37
📒 Files selected for processing (2)
src/main/java/org/sasanlabs/service/vulnerability/ldapInjection/LDAPInjectionVulnerability.javasrc/main/resources/i18n/messages.properties
| description = "LDAP_LEVEL_4_SECURE", | ||
| payload = "LDAP_PAYLOAD_LEVEL_4") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Missing i18n key for updated Level 4 description label.
Line 273 switches to LDAP_LEVEL_4_SECURE, but there is no matching key in src/main/resources/i18n/messages.properties. This will surface as an unresolved label/fallback in metadata consumers.
Suggested fix
# src/main/resources/i18n/messages.properties
+LDAP_LEVEL_4_SECURE=This level demonstrates secure LDAP input handling. User input is encoded before query construction, preventing LDAP injection through filter manipulation.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| description = "LDAP_LEVEL_4_SECURE", | |
| payload = "LDAP_PAYLOAD_LEVEL_4") | |
| LDAP_LEVEL_4_SECURE=This level demonstrates secure LDAP input handling. User input is encoded before query construction, preventing LDAP injection through filter manipulation. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/org/sasanlabs/service/vulnerability/ldapInjection/LDAPInjectionVulnerability.java`
around lines 273 - 274, The description key LDAP_LEVEL_4_SECURE used in the
LDAPInjectionVulnerability class is missing from the i18n messages.properties
file. Add the missing internationalization key LDAP_LEVEL_4_SECURE to the
messages.properties file with an appropriate value for the Level 4 secure LDAP
injection description. This will ensure the label is properly resolved when
consumed by metadata consumers and prevent fallback behavior.
| LDAP_INJECTION_LEVEL_5_HINT_3=Try supplying the project's default testing filter payload 'I*)(uid=*, *' in the username field with any arbitrary password to execute the bypass. | ||
| LDAP_INJECTION_LEVEL_5_PAYLOAD_DESCRIPTION=Blind LDAP logical filtering payload designed to bypass structural password validation checks entirely. | ||
| LDAP_INJECTION_LEVEL_5_PAYLOAD_VALUE=*)(uid=*, * No newline at end of file |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Payload example mismatch between hint and payload value.
Line 628 shows I*)(uid=*, * while Line 630 uses *)(uid=*, *. Please align these so users aren’t given contradictory payload guidance.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/resources/i18n/messages.properties` around lines 628 - 630, The
LDAP_INJECTION_LEVEL_5_HINT_3 property and LDAP_INJECTION_LEVEL_5_PAYLOAD_VALUE
property contain mismatched payloads. The hint shows I*)(uid=*, * while the
payload value shows *)(uid=*, * (missing the leading I character). Align these
two properties by ensuring both contain the same payload value so users receive
consistent guidance. Verify which version is the correct payload and update
whichever property is incorrect to match.
| @AttackVector( | ||
| vulnerabilityExposed = VulnerabilityType.LDAP_INJECTION, | ||
| description = "LDAP_LEVEL_4_INPUT_SANITIZATION", | ||
| description = "LDAP_LEVEL_4_SECURE", |
| LDAP_INJECTION_LEVEL_1_HINT_2_2=Instead of looking for a specific user or a specific pattern prefix, consider what input matches absolutely any string. | ||
| LDAP_INJECTION_LEVEL_1_HINT_2_3=Inputting a single standalone asterisk character turns the evaluated query into (uid=*), compelling the system to match and return all accounts. | ||
| LDAP_INJECTION_LEVEL_1_PAYLOAD_2_DESCRIPTION=Standalone universal wildcard payload designed to dump the entire user database directory. | ||
| LDAP_INJECTION_LEVEL_1_PAYLOAD_2_VALUE=* |
There was a problem hiding this comment.
this is not a valid payload. can you please find a valid payload?
preetkaran20
left a comment
preetkaran20
left a comment
There was a problem hiding this comment.
This is not the right change @An16og. I would suggest look at code, start the app and try to exploit the vulnerabilities and then only raise the PR.
|
This PR has had no activity for 30 days, so it's been marked stale. If you're still working on it, push a commit or leave a comment and this label will clear. Otherwise it'll close automatically in 60 days — no hard feelings, and you (or anyone else) are welcome to reopen or resubmit against current main whenever you're ready. |
3 participants
Summary by CodeRabbit