Details
-
Type:
Improvement
-
Status: Closed
-
Priority:
Major
-
Resolution: Fixed
-
Affects Version/s: 3.2
-
Fix Version/s: EE-3.2.0.GA, 3.3
-
Component/s: Framework, ICE-Components
-
Labels:None
-
Environment:Security
-
Assignee Priority:P2
Description
This is a specific case opened up as part of a detailed analysis (ICE-8771) of a Veracode security report submitted by a customer.
The reported issue was: "External Control of File Name or Path"
The details provided by Veracode were:
This call to java.lang.ClassLoader.getResourceAsStream() contains a path manipulation flaw. The argument to the function is a filename constructed using user-supplied input. If an attacker is allowed to specify all or part of the filename, it may be possible to gain unauthorized access to files on the server, including those outside the webroot, that would be normally be inaccessible to end users. The level of exposure depends on the effectiveness of input validation routines, if any. The first argument to getResourceAsStream() contains tainted data from the variable path. The tainted data originated from an earlier call to javax.servlet.http.HttpServletRequest.getPathInfo. Validate all user-supplied input to ensure that it conforms to the expected format, using centralized data validation routines when possible. When using black lists, be sure that the sanitizing routine performs a sufficient number of iterations to remove all instances of disallowed characters.
The relevant class is:
com.icesoft.faces.webapp.CompatResourceServlet
void service(javax.servlet.http.HttpServletRequest, javax.servlet.http.HttpServletResponse)
The task is to review the and attempt to ensure, if possible, that input from the user is sanitized before being used to generate path names.
The reported issue was: "External Control of File Name or Path"
The details provided by Veracode were:
This call to java.lang.ClassLoader.getResourceAsStream() contains a path manipulation flaw. The argument to the function is a filename constructed using user-supplied input. If an attacker is allowed to specify all or part of the filename, it may be possible to gain unauthorized access to files on the server, including those outside the webroot, that would be normally be inaccessible to end users. The level of exposure depends on the effectiveness of input validation routines, if any. The first argument to getResourceAsStream() contains tainted data from the variable path. The tainted data originated from an earlier call to javax.servlet.http.HttpServletRequest.getPathInfo. Validate all user-supplied input to ensure that it conforms to the expected format, using centralized data validation routines when possible. When using black lists, be sure that the sanitizing routine performs a sufficient number of iterations to remove all instances of disallowed characters.
The relevant class is:
com.icesoft.faces.webapp.CompatResourceServlet
void service(javax.servlet.http.HttpServletRequest, javax.servlet.http.HttpServletResponse)
The task is to review the and attempt to ensure, if possible, that input from the user is sanitized before being used to generate path names.
Issue Links
- blocks
-
ICE-8771
SECURITY: Potential security improvements related to findings from Veracode security scan
-
- Closed
-
Activity
- All
- Comments
- History
- Activity
- Remote Attachments
- Subversion
Hello Deryk,
This fix produce Null pointer exception on URL's like "http://host/url-pattern", "http://host/url-pattern/". On this url's getPathInfo() method return null,
and throw null pointer exception here "(path.contains("..")) {".
Thanks.
Show
Kairat Rakhimov
added a comment - - edited Hello Deryk,
This fix produce Null pointer exception on URL's like "http://host/url-pattern", "http://host/url-pattern/". On this url's getPathInfo() method return null,
and throw null pointer exception here "(path.contains("..")) {".
Thanks.
Thanks for catching this. Since 3.2 is no longer under active development, I've opened a new JIRA (ICE-9942) targeted for the next release and applied a patch.
Show
Deryk Sinotte
added a comment - Thanks for catching this. Since 3.2 is no longer under active development, I've opened a new JIRA ( ICE-9942 ) targeted for the next release and applied a patch.
The CompatResourceServlet has this code which the Verascan picks up a potentially dangerous:
... private static final String BASE_PATH = "com/icesoft/faces/resources"; ... String path = httpServletRequest.getPathInfo(); final InputStream in = loader.getResourceAsStream(BASE_PATH + path); ...The challenge is to "sanitize" the path variable so that getResourceAsStream isn't allowed to return anything it isn't supposed to. The JavaDoc for getPathInfo():
"Returns any extra path information associated with the URL the client sent when it made this request. The extra path information follows the servlet path but precedes the query string and will start with a "/" character."
All I can think of being a problem here is someone crafting the request so that the path ends up containing some directory manipulation (e.g. "/../../../..") that might allow access to something outside of what the BASE_PATH sets. The method isn't going to execute anything like a script.
Consulted with Ted and this appears to be very low risk. We could just reject any strings containing ".." and log a security warning. I suspect that there is no security hole here, but rejecting ".." would be the safest thing to do.
So I've added a check to the path value that will check for paths with ".." in them and if it detects one:
Not sure whether this will satisfy the Veracode scanning or not but should satisfy the spirit of the issue.