Skip to content

fix: Success if only Smith earn most salary #1744

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Jun 1, 2024
Merged

fix: Success if only Smith earn most salary #1744

merged 5 commits into from
Jun 1, 2024

Conversation

FrancoisCapon
Copy link
Contributor

Hello,

Some students succeed the SQL (intro) > lesson 9 by updating all the salaries of employees!

With this update it's no more possible ; I also changed the behavor of the response :

  • the data returned are always the data employees ordered by salary desc (confidentiality injection remain in the previous lesson)
  • the injection is commited only in success case
    • employees data remain clear
    • solutions too for all students

and of course:

  • this pull request will not pass the current SqlInjectionLesson9Test 🛑
  • but the updated test class SqlInjectionLesson9Test yes 🆗
./mvnw test -Dtest="org.owasp.webgoat.lessons.sqlinjection.introduction.SqlInjectionLesson9Test"
...

[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running org.owasp.webgoat.lessons.sqlinjection.introduction.SqlInjectionLesson9Test
...
[INFO] Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 8.535 s -- in org.owasp.webgoat.lessons.sqlinjection.introduction.SqlInjectionLesson9Test
[INFO] 
[INFO] Results:
[INFO] 
[INFO] Tests run: 5, Failures: 0, Errors: 0, Skipped: 0

@FrancoisCapon FrancoisCapon changed the title fix: Success only if Smith earn most salary fix: Success if only Smith earn most salary Feb 10, 2024
@nbaars nbaars force-pushed the fix-increase-only-smith-salary branch from 5d6120f to 97a44e7 Compare March 17, 2024 10:00
@zubcevic
Copy link
Collaborator

zubcevic commented Jun 1, 2024

Although changing all is also changing the one, this makes it a bit more challenging. Even tough an attacker will probably go for the easy way of changing all. There were some issues with the slow macOS runner in GitHub, so after a rebase on main it should now pass. Thanks for the contributions.

@zubcevic zubcevic merged commit 3134f18 into WebGoat:main Jun 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@FrancoisCapon @zubcevic