4

Following are checkmarx issue details Unrestricted File Upload

Source Object : req (Line No - 39)

target Object : getInputStream (Line No -41)

    public class JWTLoginFilter extends AbstractAuthenticationProcessingFilter
{

    //...
38 public Authentication attemptAuthentication(HttpServletRequest req, HttpServletResponse res)
39            throws AuthenticationException, IOException, ServletException
40    {
41        Entitlements creds = new ObjectMapper().readValue(req.getInputStream(), Entitlements.class);

        return getAuthenticationManager().authenticate(
                new UsernamePasswordAuthenticationToken(creds.getId(), "", Collections.emptyList()));
    }
    //...
}

request objects get highlighted in checkmarx tool -

How do I properly validate, filter, escape, and/or encode user-controllable input to pass a Checkmarx scan?

StackOverFlow
  • 4,486
  • 12
  • 52
  • 87
  • your question is a little bit broad but I will assume you are referring to the request object specifically. what to validate or filter or escape depends on which property of the HttpServletRequest you are using and processing. mind if you share some more code? – securecodeninja Oct 01 '20 at 16:52
  • @RomanCanlas More code snippet added for reference. – StackOverFlow Oct 01 '20 at 19:43
  • 1
    with respect to the context of the code, i think this is a false positive. the obvious source here is request.getHeader("Authorization") where Checkmarx is suspicious of to be an entry point for malicious input, but the token doesn't appear to be rendered on a page where it would cause XSS – securecodeninja Oct 01 '20 at 20:26
  • @RomanCanlas - token = Encode.forHtml(token); worked and "Reflected XSS All Clients" issues resolved. I have updated code for "Unrestricted File Upload" checkmarx issue. – StackOverFlow Oct 05 '20 at 07:46
  • 1
    You could try checking that the content-type header is application/json before you use the input stream, but without more information about what Checkmarx looks for it's hard to say. – tgdavies Oct 10 '20 at 09:36
  • @tgdavies -How to check ? inputstream = httpServletRequest.getInputStream - – StackOverFlow Oct 13 '20 at 09:09
  • 1
    if (req.getHeader("Content-Type") == null || !req.getHeader("Content-Type").startsWith("application/json")} { // return a 400 result } But really you need to find out exactly what Checkmarx expects. – tgdavies Oct 13 '20 at 09:24
  • @tgdavies Thanks for you input. I have added check for content length , content type check with "application/xml;charset=UTF-8" as http request sending content type. & Posted 2nd answer. Let's see how checkmarx scan code. – StackOverFlow Oct 13 '20 at 17:35

5 Answers5

1

This worked for me - checkmarx pass this high vulnerability

I used combination of @reflexdemon ans and @tgdavies comment

@Override
public Authentication attemptAuthentication(HttpServletRequest req, HttpServletResponse res)
        throws IOException
{
    int len = req.getContentLength();
    len = Integer.parseInt(Encode.forHtml(String.valueOf(len)));
    String type = req.getContentType();
    type =  Encode.forHtml(type);
    Entitlements creds;
    if(len == INPUT_LENGTH && type.equals(MIMETYPE_TEXT_PLAIN_UTF_8)) {
        creds = new ObjectMapper().readValue(req.getReader().lines().collect(Collectors.joining(System.lineSeparator())), Entitlements.class);
    }else{
        creds = new Entitlements();
    }

    return getAuthenticationManager().authenticate(
            new UsernamePasswordAuthenticationToken(creds.getId(), "", Collections.emptyList()));
}
StackOverFlow
  • 4,486
  • 12
  • 52
  • 87
1

Below solutions worked for me for checkmarx scan. In case of stored xss I used HtmlUtils.escapeHtmlContent(String)

In case if we want to sanitize the bean classes used in @requestbody we have to use

Jsoup.clean(StringEscapeUtils.escapHtml4(objectMapper.writeValueAsString(object)), Whitelist.basic());

This has solved the checkmarx vulnerability issues for me

Abdur Rahman
  • 1,420
  • 1
  • 21
  • 32
Loki
  • 11
  • 2
0

Sometimes, we can trick the tool with a level of indirection. Can you try the below and see if that fixes your problem,

Replace:

Entitlements creds = new ObjectMapper().readValue(req.getInputStream(), Entitlements.class);

With,

Entitlements creds = new ObjectMapper().readValue(req.getReader().lines().collect(Collectors.joining(System.lineSeparator())), Entitlements.class);
reflexdemon
  • 836
  • 8
  • 21
  • Let me try your solution. Just for your information - checkmarx checks entire flow of input parameters of function. In your solution validation/encoding/filtering not used – StackOverFlow Oct 06 '20 at 19:17
  • Could you please explain, how level of indirection will trick the tool – StackOverFlow Oct 07 '20 at 11:59
  • 1
    Most code scan tools depend on standard patterns and detect errors based on known patterns. In your case sending the raw input stream to a method is a sure red flag. The expectation is you parse the input before you start parsing the data. These indirection, will be a non standard pattern and thus allow you pass the tests. We use this only if we know for sure it is a false positive. – reflexdemon Oct 07 '20 at 15:01
  • Did it fix the issue? – reflexdemon Oct 07 '20 at 20:13
  • Not yet applied ur code changes. waiting for my earlier code change report. FYI - String inputStreamStr = IOUtils.toString(req.getInputStream(), StandardCharsets.UTF_8.name()); inputStreamStr = SecurityUtil.sanitizeObject(inputStreamStr, String.class); InputStream inputStream = new ByteArrayInputStream(inputStreamStr.getBytes()); Entitlements creds = new ObjectMapper().readValue(inputStream, Entitlements.class); – StackOverFlow Oct 08 '20 at 14:28
  • All the very best! – reflexdemon Oct 08 '20 at 17:56
  • Accepted your solution. Thanks :) updated answer https://stackoverflow.com/a/64249625/297907 – StackOverFlow Oct 14 '20 at 05:05
0

You code can be refactored to be like this:

// Negative
public class JWTLoginFilter extends AbstractAuthenticationProcessingFilter {

    public Authentication attemptAuthentication(HttpServletRequest req, HttpServletResponse res)
            throws AuthenticationException, IOException, ServletException {

        if (req.getContentLength() > MAX_REQUEST_SIZE) {
            throw new IOException("request body size too big!");
        }

        Entitlements creds = new ObjectMapper().readValue(req.getInputStream(), Entitlements.class);

        return getAuthenticationManager()
                .authenticate(new UsernamePasswordAuthenticationToken(creds.getId(), "", Collections.emptyList()));
    }

}

You can use use getContentLength as a validator. While by default CxSAST 9.3 is not able to detect this validator. You can override the Java_Low_Visibility/Unrestricted_File_Upload query by the content from this file: https://github.com/checkmarx-ts/CxQL/blob/master/Java/Java_Low_Visibility/Unrestricted_File_Upload.txt

Other validators are also supported, getSize, getFileSize. You can also use MultipartConfig annotation with maxRequestSize. Or use multipart-config max-request-size in web.xml.

-2

Seems like the scanner found an XSS vulnerability in your code.

From OWASP's Cross-site Scripting (XSS) page:

Cross-Site Scripting (XSS) attacks are a type of injection, in which malicious scripts are injected into otherwise benign and trusted websites. XSS attacks occur when an attacker uses a web application to send malicious code, generally in the form of a browser side script, to a different end user. Flaws that allow these attacks to succeed are quite widespread and occur anywhere a web application uses input from a user within the output it generates without validating or encoding it.

To learn in-depth how to avoid Cross-site Scripting vulnerabilities, it is very recommended to go over OWASP's XSS (Cross-Site Scripting) Prevention Cheat Sheet page. There are some sanitizer options listed there, and you can choose according to the specific language and relevant use.

Good luck.

yaloner
  • 715
  • 2
  • 6
  • 19