Excellent points, and I agree that consistency is very important. So what's the next step?
For discussion purposes, here is an initial cut at the methods that produce untrusted data...
javax.servlet.ServletRequest.getInputStream()
javax.servlet.ServletRequest.getParameter(java.lang.String)
javax.servlet.ServletRequest.getParameterNames()
javax.servlet.ServletRequest.getParameterValues(java.lang.String)
javax.servlet.ServletRequest.getParameterMap()
javax.servlet.httpCookie.getName()
javax.servlet.httpCookie.getValue()
javax.servlet.http.HttpServletRequest.getHeader(java.lang.String)
javax.servlet.http.HttpServletRequest.getHeaderNames()
javax.servlet.http.HttpServletRequest.getHeaders(java.lang.String)
javax.servlet.http.HttpServletRequest.getMethod() -- this one has limited characters
javax.servlet.http.HttpServletRequest.getRequestURI()
javax.servlet.http.HttpServletRequest.getRequestURL()
javax.servlet.http.HttpServletRequest.getRequestedSessionId()
javax.servlet.http.HttpServletRequest.getPart(java.lang.String)
javax.servlet.http.HttpServletRequest.getParts()
javax.servlet.http.HttpServletRequest.getQueryString()
javax.servlet.http.HttpServletRequest.getCookies()
javax.servlet.http.HttpServletRequestWrapper.getHeader(java.lang.String)
javax.servlet.http. HttpServletRequestWrapper.getHeaderNames()
javax.servlet.http. HttpServletRequestWrapper.getHeaders(java.lang.String)
javax.servlet.http. HttpServletRequestWrapper.getMethod() -- this one has limited characters
javax.servlet.http. HttpServletRequestWrapper.getRequestURI()
javax.servlet.http. HttpServletRequestWrapper.getRequestURL()
javax.servlet.http. HttpServletRequestWrapper.getRequestedSessionId()
javax.servlet.http. HttpServletRequestWrapper.getPart(java.lang.String)
javax.servlet.http. HttpServletRequestWrapper.getParts()
javax.servlet.http. HttpServletRequestWrapper.getQueryString()
javax.servlet.http. HttpServletRequestWrapper.getCookies()
javax.servlet.http.Part.getHeader(java.lang.String)
javax.servlet.http.Part.getHeaderNames()
javax.servlet.http.Part.getHeaders(java.lang.String)
javax.servlet.http.Part.getInputStream()
javax.servlet.http.Part.getName()
javax.servlet.http.Part.getSubmittedFileName()
javax.servlet.http.HttpUtils.getRequestURL(javax.servlet.http.HttpServletRequest) - deprecated
javax.servlet.http.HttpUtils.parsePostData(int, javax.servlet.ServletInputStream) - deprecated
javax.servlet.http.HttpUtils.parseQueryString(java.lang.String) - deprecated
ServletInputStream.readLine(byte[],int,off,len) - probably best to warn in the class overview too
What's more effective, a generic warning like "This method returns untrusted data from the browser that may include attacks. Be sure to canonicalize, validate, and properly encode this data before using"? Or more specific warnings that go into the likely attacks?
I was also thinking that this could be an annotation for use by static analysis tools -- @untrusted. But I'm not sure the status of JSR 305/308 on this.
--Jeff
-----Original Message-----
From: Mark Thomas [mailto:markt_at_apache.org]
Sent: Monday, March 04, 2013 3:41 AM
To: jsr340-experts_at_servlet-spec.java.net
Subject: [jsr340-experts] Re: About SERVLET_SPEC-57
On 04/03/2013 03:50, Jeff Williams wrote:
> Is there some downside here that I'm not seeing? Why wouldn't we try
> to make it easier for developers to write secure apps?
I have no objection to a general warning that client supplied data should not be trusted.
I do object to only adding a warning to the Javadoc for only some client supplied data since that could easily lead developers to conclude that any data that doesn't have the warning is safe to use.
> Actually, why wouldn't we add an indicator of some kind to all the
> sources of untrusted data? It's quite confusing actually, since some
> of the HTTP request methods provide untrusted data and some do not.
I'd be fine with that too.
> And this situation is different, since the likelihood that someone
> will take this input and turn it into a file path is so high. A
> developer might do almost anything with an HTTP request parameter, but
> the name of an uploaded file? They could easily make a bad mistake
> here.
It is just as easy - and just as likely - that an HTTP request parameter will be used directly in the response thereby creating an XSS issue.
My biggest concern is consistency. I don't really mind whether per method warnings are added or not but if they are added they need to be added consistently to every method where a warning is required=.
Mark
> --Jeff
>
>
> -----Original Message----- From: Mark Thomas [mailto:markt_at_apache.org]
> Sent: Sunday, February 24, 2013 1:16 PM To:
> jsr340-experts_at_servlet-spec.java.net Subject: [jsr340-experts] Re:
> About SERVLET_SPEC-57
>
> On 24/02/2013 03:02, Jeff Williams wrote:
>> There's a bit of security risk here if developers take that filename
>> and append it to a path. Attackers might send ../../../etc/passwd or
>> something. Another common bypass is if the application checks if the
>> filename endsWith( ".pdf" ) or something
>> -- the attacker can bypass by sending file.xls%00.pdf.
>>
>> Is there any restriction on the String returned from the multipart
>> request? Could we add some simple restrictions? At a minimum, could
>> we add some Javadoc describing the risks?
>
> We don't spell out the risks of using user supplied data directly
> anywhere else in the servlet spec so I don't see a need to do so here.
> The Javadoc should makde clear (although the method name is already
> about as clear as it gets) that the filename is provided by the
> client.
>
> Mark
>
>
>>
>> Thanks,
>>
>> --Jeff
>>
>>
>> -----Original Message----- From: Shing Wai Chan
>> [mailto:shing.wai.chan_at_oracle.com] Sent: Friday, February 22, 2013
>> 7:22 PM To: jsr340-experts_at_servlet-spec.java.net Subject:
>> [jsr340-experts] About SERVLET_SPEC-57
>>
>> I am looking at http://java.net/jira/browse/SERVLET_SPEC-57 (" Add
>> getFileName() method to javax.servlet.http.Part")
>>
>> It is a good to have an API to return the file name. The question is
>> the method name.
>>
>> Is the following good enough?
>>
>> In javax.servlet.http.Part, add the following API
>>
>> /** * Return the name of the submitted file. * @return The name of
>> the submitted file as a String */ String getSubmittedFileName()
>>
>>
>> Thanks. Shing Wai Chan
>>
>