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

VariantURLQuery throwing exceptions on active scan #1848

Closed
thilina27 opened this issue Aug 24, 2015 · 5 comments · Fixed by #2456
Closed

VariantURLQuery throwing exceptions on active scan #1848

thilina27 opened this issue Aug 24, 2015 · 5 comments · Fixed by #2456
Assignees
Milestone

Comments

@thilina27
Copy link

When scanning this error occurs.

Error like this occurs for different scan rules

769590 [ZAP-ActiveScanner-1] ERROR org.zaproxy.zap.extension.ascanrules.TestServerSideInclude - Error occurred while scanning with variant org.parosproxy.paros.core.scanner.VariantURLQuery
java.lang.IllegalArgumentException: URLDecoder: Illegal hex characters in escape (%) pattern - For input string: "{p"
at java.net.URLDecoder.decode(URLDecoder.java:192)
at org.parosproxy.paros.core.scanner.AbstractPlugin.getURLDecode(Unknown Source)
at org.parosproxy.paros.core.scanner.VariantURLQuery.getUnescapedValue(Unknown Source)
at org.parosproxy.paros.core.scanner.VariantAbstractQuery.setParams(Unknown Source)
at org.parosproxy.paros.core.scanner.VariantURLQuery.setMessage(Unknown Source)
at org.parosproxy.paros.core.scanner.AbstractAppParamPlugin.scan(Unknown Source)
at org.parosproxy.paros.core.scanner.AbstractPlugin.run(Unknown Source)
at java.lang.Thread.run(Thread.java:745)

@psiinon
Copy link
Member

psiinon commented Aug 24, 2015

Does the scan carry on anyway?
I was fairly sure that these were benign...

@thilina27
Copy link
Author

yes scan is carry on. But dose it skip some scans ?

@thc202
Copy link
Member

thc202 commented Aug 26, 2015

Yes, some query parameters are not tested because of the exception(s).
The issue is related to #1801 (the class VariantURLQuery expects the query parameters escaped but they are not).
Most likely fixing the other issue will fix this one.

@thc202 thc202 changed the title VariantURLQuery throwing URIExceptions on active scan VariantURLQuery throwing exceptions on active scan Aug 26, 2015
@thc202 thc202 added the bug label Aug 26, 2015
@thc202
Copy link
Member

thc202 commented Nov 9, 2015

Would you mind if the issue is reopened? The issue was not yet fixed and closing it might give that impression.

@thilina27 thilina27 reopened this Nov 9, 2015
@thc202 thc202 added this to the 2.4.x milestone Jan 7, 2016
@thc202 thc202 self-assigned this Mar 31, 2016
thc202 added a commit to thc202/zaproxy that referenced this issue May 4, 2016
Parse form/query parameters in encoded form, so they are correctly split
into name/value pairs even if the names and/or values have separator
characters (normally, '&' and '='), also allow to keep all parameters,
so requests that require multiple values with same name (e.g. arrays)
are still sent with all of them (instead of just one).
When rebuilding the query component of the URL, during active scan, make
sure to also encode parameters' names not just its values, otherwise the
query could be malformed if the names contain characters that should be
encoded (common when dealing with multiple vales which might use the
characters '[' and ']'), leading to exceptions and failing to properly
attack those parameters. (The same logic applies for form parameters.)

More detailed changes:
 - Variant, document the method getParamList();
 - VariantAbstractQuery:
  - Add method, getEscapedName(HttpMessage, String), to encode/escape
  parameter's name;
  - Add method, setParameters(int, List<NameValuePair>), to allow
  extending classes to set (and finally send) all parameters even if
  they have the same name;
  - Deprecate the method setParams(int, Map<String, String>) in favour
  of the new method.
 - VariantFormQuery and VarianURLQuery, change to obtain all the
 parameters using a new method in Session class and to set all of them
 using the new method of the class VariantAbstractQuery;
 - Session:
  - Add method, getParameters(HttpMessage, HtmlParameter.Type), that
  allows to obtain (all) URL or form parameters from a message;
  - Change method getUrlParams(URI) to parse the query in escaped form
   (and use the new method added to ParameterParser, which decodes the
   parameters names and values when parsing, as expected by callers);
 - Add interface NameValuePair and default implementation, class
 DefaultNameValuePair;
 - ParameterParser, add methods getParameters(...) and
 parseParameters(String) which return List<NameValuePair>;
 - StandardParameterParser:
  - Add implementations of the methods added to the interface
  ParameterParser;
  - Change method getParams, getTreePath and getAncestorPath to use
  parseParameters(String) when parsing the (now escaped) query, so the
  parameters are correctly separated (and decoded).

Fix zaproxy#1801 - URL StandardParameterParser not working correctly with
QueryString
Fix zaproxy#1848 - VariantURLQuery throwing exceptions on active scan
Fix zaproxy#2153 - 2.4.3 failed parse the POST Data containts bracket([])
thc202 added a commit to thc202/zaproxy that referenced this issue May 5, 2016
Parse form/query parameters in encoded form, so they are correctly split
into name/value pairs even if the names and/or values have separator
characters (normally, '&' and '='), also allow to keep all parameters,
so requests that require multiple values with same name (e.g. arrays)
are still sent with all of them (instead of just one).
When rebuilding the query component of the URL, during active scan, make
sure to also encode parameters' names not just its values, otherwise the
query could be malformed if the names contain characters that should be
encoded (common when dealing with multiple vales which might use the
characters '[' and ']'), leading to exceptions and failing to properly
attack those parameters. (The same logic applies for form parameters.)

More detailed changes:
 - Variant, document the method getParamList();
 - VariantAbstractQuery:
  - Add method, getEscapedName(HttpMessage, String), to encode/escape
  parameter's name;
  - Add method, setParameters(int, List<NameValuePair>), to allow
  extending classes to set (and finally send) all parameters even if
  they have the same name;
  - Deprecate the method setParams(int, Map<String, String>) in favour
  of the new method.
 - VariantFormQuery and VarianURLQuery, change to obtain all the
 parameters using a new method in Session class and to set all of them
 using the new method of the class VariantAbstractQuery;
 - Session:
  - Add method, getParameters(HttpMessage, HtmlParameter.Type), that
  allows to obtain (all) URL or form parameters from a message;
  - Change method getUrlParams(URI) to parse the query in escaped form
   (and use the new method added to ParameterParser, which decodes the
   parameters names and values when parsing, as expected by callers);
 - Add interface NameValuePair and default implementation, class
 DefaultNameValuePair;
 - ParameterParser, add methods getParameters(...) and
 parseParameters(String) which return List<NameValuePair>;
 - StandardParameterParser:
  - Add implementations of the methods added to the interface
  ParameterParser;
  - Change method getParams, getTreePath and getAncestorPath to use
  parseParameters(String) when parsing the (now escaped) query, so the
  parameters are correctly separated (and decoded).

Fix zaproxy#1801 - URL StandardParameterParser not working correctly with
QueryString
Fix zaproxy#1848 - VariantURLQuery throwing exceptions on active scan
Fix zaproxy#2153 - 2.4.3 failed parse the POST Data containts bracket([])
@lock
Copy link

lock bot commented Feb 1, 2020

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

@lock lock bot locked and limited conversation to collaborators Feb 1, 2020
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