Skip to content
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

fix(jwt-auth): real_payload was overridden by malicious payload #10982

Merged
merged 6 commits into from
Mar 7, 2024
Merged

fix(jwt-auth): real_payload was overridden by malicious payload #10982

merged 6 commits into from
Mar 7, 2024

Conversation

jaspertian
Copy link
Contributor

@jaspertian jaspertian commented Feb 29, 2024 •
edited
Loading

Description

Fixes #10967

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@Gallardot Gallardot changed the title fix: jwt-auth plugin : real_payload was overridden by malicious payload fix(jwt-auth): real_payload was overridden by malicious payload Feb 29, 2024
Gallardot
Gallardot requested changes Mar 1, 2024
View reviewed changes
Copy link
Member

@Gallardot Gallardot left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please refer jwt-auth.t and add some UT. The rest LGTM.

@monkeyDluffy6017
Copy link
Contributor

Test cases are needed

@jaspertian
Copy link
Contributor Author

Test cases are needed

Got it.

monkeyDluffy6017
monkeyDluffy6017 reviewed Mar 4, 2024
View reviewed changes
t/plugin/jwt-auth4.t Outdated

__DATA__

=== TEST 24: verify the real_payload's value (key & exp) is not overridden by malicious payload
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The index should be 1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I got some troube when building local test env.

I'm trying to fix it and will feedback asap.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I re-implement the test case and verified in local test env, please check it.

Any problem , pls contact me. thx!

Gallardot
Gallardot approved these changes Mar 7, 2024
View reviewed changes
Copy link
Member

@Gallardot Gallardot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thank you for your contribution!

@monkeyDluffy6017 monkeyDluffy6017 merged commit f932fbe into apache:master Mar 7, 2024
45 checks passed
@jaspertian jaspertian deleted the fix-10967 branch March 8, 2024 02:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@monkeyDluffy6017 monkeyDluffy6017 monkeyDluffy6017 approved these changes

@Gallardot Gallardot Gallardot approved these changes

Assignees
No one assigned
Labels
approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

bug: 插件 jwt-auth 的 function get_real_payload 存在 key 和 exp 被恶意修改的可能
3 participants
@jaspertian @monkeyDluffy6017 @Gallardot