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.

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

Cannot detect injection vulnerabilities on null value JSON objects #6469

Closed
gitveio opened this issue Mar 1, 2021 · 9 comments · Fixed by #6539
Closed

Cannot detect injection vulnerabilities on null value JSON objects #6469

gitveio opened this issue Mar 1, 2021 · 9 comments · Fixed by #6539
Assignees
Milestone

Comments

@gitveio
Copy link

gitveio commented Mar 1, 2021

As the title describes, if the JSON of the request content is:
{"name":null}
The attribute of name will not be tested at all. In fact, the name has a SQL injection problem.

But if you modify the JSON to:
{"name":""}
In this case, the problem will be detected. I am not sure if this is a real problem or just a configuration problem.
Can anyone help me, thanks

@gitveio gitveio added the bug label Mar 1, 2021
@thc202 thc202 added enhancement and removed bug labels Mar 1, 2021
@thc202
Copy link
Member

thc202 commented Mar 1, 2021

Right, null values are not tested, I guess they aren't because ZAP does not know which type is supposed to be there (e.g. is it an object, an array, or a string?). It could default to string...

@psiinon
Copy link
Member

psiinon commented Mar 1, 2021

+1 for defaulting to string :)

@gitveio
Copy link
Author

gitveio commented Mar 1, 2021

Understand, thank you for your reply.

@gitveio
Copy link
Author

gitveio commented Mar 9, 2021

Can I deal with this one, and if so, whether dealing with all null values will significantly reduce the test speed. If it is me, an option switch may be added to "input vectors", which is not checked by default. Is this acceptable, or some other ideas.

@kingthorin
Copy link
Member

I like the idea of making it optional, I'm not sure if it should be opt-in or opt-out. How often do uou see null values in json vs. empty strings?

It would be similar to the changes done here:

@thc202
Copy link
Member

thc202 commented Apr 1, 2021

@gitveio are you still working on this issue?

@gitveio
Copy link
Author

gitveio commented Apr 13, 2021

Sorry, I'm not working on it.

@gitveio gitveio closed this as completed Apr 13, 2021
@thc202
Copy link
Member

thc202 commented Apr 13, 2021

OK, no worries. Reopening, still worth addressing.

@thc202 thc202 reopened this Apr 13, 2021
@thc202 thc202 self-assigned this Apr 13, 2021
@thc202 thc202 added this to the 2.11.0 milestone Apr 13, 2021
thc202 added a commit to thc202/zaproxy that referenced this issue Apr 13, 2021
Change JSON variant to handle null values as strings.
Add option to enable the new behaviour, disabled by default.

Fix zaproxy#6469.

Signed-off-by: thc202 <thc202@gmail.com>
kosap pushed a commit to kosap/zaproxy that referenced this issue Jun 3, 2021
Change JSON variant to handle null values as strings.
Add option to enable the new behaviour, disabled by default.

Fix zaproxy#6469.

Signed-off-by: thc202 <thc202@gmail.com>
@github-actions
Copy link

This issue 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 Jul 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging a pull request may close this issue.

4 participants