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

Option to encode cookie values #8203

Merged
merged 1 commit into from
Nov 22, 2023
Merged

Option to encode cookie values #8203

merged 1 commit into from
Nov 22, 2023

Conversation

psiinon
Copy link
Member

@psiinon psiinon commented Nov 21, 2023

This changes the default behaviour, but I think for the better.
Help PR will follow.

Active Scan Input Vectors Options dialog

Screenshot 2023-11-21 at 12 16 09

@psiinon
Copy link
Member Author

psiinon commented Nov 21, 2023

Looking at the failing tests now..

@kingthorin
Copy link
Member

🤦‍♂️ spotless

@thc202 thc202 added this to the 2.15.0 milestone Nov 21, 2023
@psiinon
Copy link
Member Author

psiinon commented Nov 21, 2023

ugh, did that before, so that must be a problem with the properties file :/

@psiinon psiinon mentioned this pull request Nov 21, 2023
Merged
@psiinon
Copy link
Member Author

psiinon commented Nov 21, 2023

Help PR: zaproxy/zap-core-help#536

@psiinon
Copy link
Member Author

psiinon commented Nov 21, 2023

All tests passing now 😉

@kingthorin
Copy link
Member

Hmm see the help PR before my comments here

@psiinon
Copy link
Member Author

psiinon commented Nov 21, 2023

Yeah, looks like this is a misunderstanding.
Its a global option, encode cookie values or dont encode them.
I think the previous way of always encoding them was probably wrong.

I'm not sure attacking both encoded and unencoded makes a lot of sence (from a number of requests point of view), but I'm willing to be convinced otherwise.
I would rather have an initialisation step when we inject non alphanumberic values into a cookie and see if they are reflected encoded or not.
But that would be a future change.

@kingthorin
Copy link
Member

kingthorin commented Nov 21, 2023 •
edited

Okay, I'm good with this as-is.

I don't think doing a pre-condition check is feasible, you'd have to find where it's set and then used, etc. What if it isn't reflected, what if your input isn't accepted as a valid value, etc. Ouch 😢

@psiinon
Copy link
Member Author

psiinon commented Nov 21, 2023

Updated.

kingthorin
kingthorin approved these changes Nov 21, 2023
View reviewed changes
Copy link
Member

@kingthorin kingthorin left a comment

Choose a reason for hiding this comment

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

LGTM

@psiinon
Copy link
Member Author

psiinon commented Nov 22, 2023

Fixed those..

@psiinon
Option to encode cookie values
e884c9e 
Signed-off-by: Simon Bennetts <psiinon@gmail.com>
@thc202 thc202 merged commit b35fcb4 into zaproxy:main Nov 22, 2023
9 checks passed
@thc202
Copy link
Member

thc202 commented Nov 22, 2023

Thank you!

@github-actions GitHub Actions
Copy link

This PR has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked and limited conversation to collaborators Feb 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@thc202 thc202 thc202 approved these changes

@kingthorin kingthorin kingthorin approved these changes

Assignees
No one assigned
Labels
enhancement
Milestone
2.15.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@psiinon @kingthorin @thc202