Details
-
Type:
Improvement
-
Status: Closed
-
Priority:
Major
-
Resolution: Fixed
-
Affects Version/s: 3.1
-
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: "Use of Externally-Controlled Input to Select Classes or Code (Unsafe Reflection)"
The details provided by Veracode were:
_This call to java.lang.Class.forName() uses reflection in an unsafe manner=
. An attacker can specify the class name to be instantiated, which may crea=
te unexpected control flow paths through the application. Depending on how =
reflection is being used, the attack vector may allow the attacker to bypas=
s security checks or otherwise cause the application to behave in an unexpe=
cted manner. Even if the object does not implement the specified interface =
and a ClassCastException is thrown, the constructor of the user-supplied cl=
ass name will have already executed. The first argument to forName() contai=
ns tainted data from the variable type. The tainted data originated from ea=
rlier calls to javax.faces.context.ExternalContext.getRequestParameterMap, =
com.sun.faces.config.InitFacesContext$ServletContextAdapter.getRequestParam=
eterMap, javax.faces.context.ExternalContextWrapper.getRequestParameterMap,=
and com.sun.faces.context.ExternalContextImpl.getRequestParameterMap. Vali=
date the class name against a combination of white and black lists to ensur=
e that only expected behavior is produced._
The relevant class is reported to have 2 potential vulnerabilities:
com.icesoft.faces.component.util.CustomComponentUtils java.lang.Class classForName(java.lang.String)
The task is to review and attempt to ensure, if possible, that input from the user is sanitized before being used to for reflection.
The reported issue was: "Use of Externally-Controlled Input to Select Classes or Code (Unsafe Reflection)"
The details provided by Veracode were:
_This call to java.lang.Class.forName() uses reflection in an unsafe manner=
. An attacker can specify the class name to be instantiated, which may crea=
te unexpected control flow paths through the application. Depending on how =
reflection is being used, the attack vector may allow the attacker to bypas=
s security checks or otherwise cause the application to behave in an unexpe=
cted manner. Even if the object does not implement the specified interface =
and a ClassCastException is thrown, the constructor of the user-supplied cl=
ass name will have already executed. The first argument to forName() contai=
ns tainted data from the variable type. The tainted data originated from ea=
rlier calls to javax.faces.context.ExternalContext.getRequestParameterMap, =
com.sun.faces.config.InitFacesContext$ServletContextAdapter.getRequestParam=
eterMap, javax.faces.context.ExternalContextWrapper.getRequestParameterMap,=
and com.sun.faces.context.ExternalContextImpl.getRequestParameterMap. Vali=
date the class name against a combination of white and black lists to ensur=
e that only expected behavior is produced._
The relevant class is reported to have 2 potential vulnerabilities:
com.icesoft.faces.component.util.CustomComponentUtils java.lang.Class classForName(java.lang.String)
The task is to review and attempt to ensure, if possible, that input from the user is sanitized before being used to for reflection.
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
Analyzing the code involved I see that the Class.forName calls are in a single method of the com.icesoft.faces.component.util.CustomComponentUtils class - a utility class for the compat components. Digging out the usages of the method in question:
it seems that the only relevant usage is from the TabChangeListenerTag. I don't how relevant this tag is anymore as I had trouble finding an example or tutorial that makes use of it.
In any event, I added some general utility code for validating class identifiers to the EnvUtils class as it could be generally applicable for any reflection calls to insure that the string used for creating the class is a valid Java identifier. The CustomComponentUtils.classForName method will call this code whenever it attempts to reflectively create a class.