Using the encoder to avoid false positive

[Jeffrey Walton]

Hi Everyone,

I'm looking at some code that does this in a *.jsp file (forgive my
typos). Static analysis is triggering findings on it. I believe they
are mostly false positives because they are used in string compares.

if ( "Print".equalsIgnoreCase((String)params.get("ReportDisplayType") ) {
...
}

I have two questions...

(1) Does JavaScript/ECMA-262 ensure (guarantee?) that
'params.get("ReportDisplayType")' will remain inert, and not be
evaluated or become active content?

(2) How does one encode to make it safe for use in a string compare?
Would we use ESAPI.encodeForJavaScript? Or would we do something like
percent encoding since it is a string compare? Maybe something like:

String reportType =
ESAPI.encoder.encodeForURL((String)params.get("ReportDisplayType"))
if ( "Print".equalsIgnoreCase( reportType ) {
...
}

Thanks in advance.

[Kevin W. Wall]

Jeff,

Sorry it's taken so long for me to respond. Busy week with my son's fiancée visiting.

Reply inline, below.

On Mon, May 22, 2023 at 11:17 AM Jeffrey Walton <nolo...@gmail.com> wrote:

Hi Everyone,

I'm looking at some code that does this in a *.jsp file (forgive my
typos). Static analysis is triggering findings on it. I believe they
are mostly false positives because they are used in string compares.

    if ( "Print".equalsIgnoreCase((String)params.get("ReportDisplayType") ) {
        ...
    }

I think the 2 key questions are:

1. What sort of SAST warning was this flagged as?
2. What does the source-to-sink taint path look like in this particular SAST finding?

The '...' portion certainly does not seem to be vulnerable to XSS as both the parameter name and the value (presumably, that's what params.get() returns is the value for the parameter specified to the method get()) are specific, fixed strings. (Well, I take that back, I suppose there could be, based on something that you elided and just replaced with "...".)

I have two questions...

(1) Does JavaScript/ECMA-262 ensure (guarantee?) that
'params.get("ReportDisplayType")' will remain inert, and not be
evaluated or become active content?

Wait, I thought at first (since you mentioned this was in a JSP) that this was Java code? You mean it is in a <script> tag?

If this is JavaScript, I presume that 'params' has the type URLSearchParams? If so, it usually would be created something like:

    var params = new URLSearchParams( document.location.search );

And I think 'params' (and thus 'params.get("ReportdisplayType") would be find as long as your JavaScript code is thread safe. That is hard to tell from looking only at that single line, but if it is thread-safe, I think it ought not to be changing and go from safe to something unsafe.

(2) How does one encode to make it safe for use in a string compare?

Would we use ESAPI.encodeForJavaScript? Or would we do something like
percent encoding since it is a string compare? Maybe something like:

    String reportType =
ESAPI.encoder.encodeForURL((String)params.get("ReportDisplayType"))
    if ( "Print".equalsIgnoreCase( reportType ) {
        ...
    }

Safe from what? From XSS vulnerabilities? XSS vulnerabilities will only take effect when a tainted (i.e., potentially unsafe) string is rendered in the DOM. Comparing a String does no rendering so that operation is safe even if the parameter value for "ReportDisplayType" was something like "<script>alert(1)</script>" or similar. You would only need to contextually output encode it if it is used in the DOM in a manner that it might be rendered.

But now I am confused again. Is this JavaScript or Java in a JavaScript context or what? I think I'm going to need more details to make sense out of this as the example is missing a lot of important context.

If you are concerned about publicly discussing a vulnerability on an open list, just drop me an email off list and we can discuss it.

-kevin

--

Blog: https://off-the-wall-security.blogspot.com/    | Twitter: @KevinWWall | OWASP ESAPI Project co-lead
NSA: All your crypto bit are belong to us.

[Matt Seil]

There’s confusion here.  You canonicalize to make a string safe for comparisons and encode when handing input off to parsers.  

1.  Encode for outputs

2.  Canonicalize then validate inputs

If you remember those two points any confusion about esapi’s encoder should go away.  

Sent from my iPhone

On May 26, 2023, at 20:40, Kevin W. Wall <kevin....@gmail.com> wrote:



--
You received this message because you are subscribed to the Google Groups "ESAPI Project Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to esapi-project-u...@owasp.org.
To view this discussion on the web visit https://groups.google.com/a/owasp.org/d/msgid/esapi-project-users/CAOPE6PipV5e0A5Rx4uiFnxoPt5F-m8tzj0dMUFqdsbhDAVzn0g%40mail.gmail.com.