CRamsey491706 (Community Member) asked a question.
Within our Android Java application, we have logic that performs a deserialization of data into safe objects (String, ArrayList, etc). Unfortunately, despite our code throwing an exception if the deserialized object does not match the type of a provided default value (and the data being deserialized is trusted already), Veracode recognises this as a flaw and highlights it as such.
After carrying out some research on this matter in order to try to resolve the scan, rather than mitigate by design, I found a suggested solution on these forums which was put forward as an accepted and safe mechanism to deserialize data.
The relevant post where I found this solution suggested is here: https://community.veracode.com/s/question/0D52T000051XUbtSAG/how-to-fix-502deserialization-of-untrusted-data
The suggested solution linked to can be seen here: https://www.contrastsecurity.com/security-influencers/protect-your-apps-from-java-serialization-vulnerability
The solution makes sense to me. It is making use of the resolveClass() method to perform a lookahead on the type of Class before readObject() is called.
I implemented my own version of this solution, which can be seen here:
private final static List<Class<?>> safeClasses = new ArrayList<>(
Arrays.asList(String.class, Integer.class, Double.class, Long.class, Float.class,
ArrayList.class, List.class, LinkedHashMap.class, HashMap.class, Boolean.class));
public static <T> T safeReadObject(Class<?> type, long maxObjects, ByteArrayInputStream is) throws IOException, ClassNotFoundException {
// create an object input stream that checks classes and limits the number of objects to read
ObjectInputStream ois = new ObjectInputStream(is) {
private int objCount = 0;
protected Object resolveObject(Object obj) throws IOException {
if (objCount++ > maxObjects) {
throw new SecurityException("Security violation: attempt to deserialize too many objects from stream. Limit is " + maxObjects);
}
return super.resolveObject(obj);
}
protected Class<?> resolveClass(ObjectStreamClass osc) throws IOException, ClassNotFoundException {
Class<?> resolvedClass = super.resolveClass(osc);
if (resolvedClass.isArray() || resolvedClass.equals(type) || resolvedClass.equals(String.class) ||
Number.class.isAssignableFrom(resolvedClass) || safeClasses.contains(resolvedClass)) {
return resolvedClass;
}
throw new SecurityException("Security violation: attempt to deserialize unauthorized " + resolvedClass);
}
};
return (T)ois.readObject();
}
The purpose of this implementation is to throw a Security exception if the object being deserialized does not match a specifically whitelisted Class.
I have performed testing on this piece of code as well to confirm that a SecurityException is, indeed, thrown when I attempt to deserialize an unpermitted Class.
I believe I have mitigated any vulnerability here. However, Veracode is still highlighting this as a flaw. My question is, is there any alternative method I should be using? Is there a missing piece of logic from my implementation which I have failed to see? Is this now simply a false positive which I should be asking our InfoSec team to be mitigating by design?
Thanks in advance.
.png){kind=logo}
Hi @CRamsey491706 (Community Member)
The approach that you have decided to take here is something that still would require you to submit a Mitigate by design proposal. I would not select the "Potential False Positive" option here though … Veracode static analyzer did highlight a legitimate unsafe deserialization issue in your code, it is just that you are choosing to use another mitigation approach that works with your application's use case, so you want to go with "Mitigate By Design"
In general, Veracode static analyzer does not recognize and accept any custom allow list validations as strategies which can auto-resolve reported flaws. The allow list that you came up with for this particular application and its use might very well serve as a sufficient mitigation for the flaw. But, consider that when this solution is then applied to other kinds of applications and use cases, it very well might no longer serve as an suitable mitigation. This all depends on the security requirements of the application in question which is usually set by your organization's security team. Veracode static analyzer can never understand and discern use case and application intent given that all it ever has insight into is the code you upload for it to scan. That's why it is important for you to send this mitigation approach over for review and approval with your security team by using the Veracode mitigation proposal workflow.
When it comes to CWE-502 flaws reported by Veracode Static Analyzer, there are only really 2 recognized flaw auto-remediation strategies you can add to your code which Veracode analyzer can recognize upon re-scan:
Any of the above 2 approaches are viewed as stronger controls and approaches that fully address the issue of unsafe deserialization (1 is preferred above 2, as the latter requires more work and could introduce additional security concerns if not setup and managed properly). With just a custom safe read object allow list, there is still a good chance of leaving some degree of risk if not implemented properly -- and again, that's not something Veracode can safely and reliably confirm as it doesn't know about your use case, so it defaults to keeping the risk open and requiring Mitigate By Design.
Also, the thing about the current mitigation approach you are following is that even if you do limit and constrain to a list of known simple Java class types, there is still some remaining deserialization risks even within these 10 or so classes you have allowed in your helper code. Particularly with the "nestable" data structure types you are allowing such as the map and list classes, there can still remain some Denial of Service attacks that are impossible to ever mitigate and defend against if you absolutely must permit these class types. So while this approach you are currently taking certainly does help to mitigate and defend against the more severe potential risks that come with unsafe deserialization (namely remote code execution based attacks), you still need to consider if the lingering Denial of Service risks are of concern to your application and its security requirements. This ties back to what I said at the start about discussing and reviewing this with your security team on whether this is a suitable mitigation to accept in its current implementation.
If you are curious to learn more about the challenges in coming up with deserialization allow list strategies for handling Java serialization calls safely, you may want to have a read through this paper. -> https://www.nccgroup.com/globalassets/our-research/us/whitepapers/2017/june/ncc_group_combating_java_deserialization_vulnerabilities_with_look-ahead_object_input_streams1.pdf. Section 5 of this paper draws attention to the Denial of Service attacks that can still remain in serialization attempts even with this mitigation in place. The OWASP Deserialization Cheat Sheet guide is also a fantastic resource in this regard -> https://cheatsheetseries.owasp.org/cheatsheets/Deserialization_Cheat_Sheet.html#java
I hope this covers your questions and gives you a path forward, but do let me know if you should have any additional follow up here.
All the best,
Andrew Bell
References on how to get started with setting up and using HMAC in Java code.
https://www.veracode.com/blog/secure-development/message-authentication-code-mac-using-java
https://stackoverflow.com/questions/39355241/compute-hmac-sha512-with-secret-key-in-java
Take care not to hardcode your actual key and reference it instead from a secure external key manager resource, OR generate a secure random short-lived key at runtime that is then released once the operation is complete.