SKumar P226205 (Community Member) asked a question.
Earlier Code :
String[] cmd = {path, "-cp", System.getProperty("java"),file}
File dirFile = new File(System.getProperty("userdir"));
Process pro = Runtime.getRuntime().exec(cmd, null, dirFile); //Veracode throws OS Command injection in this line.
Fixed Code :
String[] cmd = {jdkPath, "-cp", System.getProperty("java"), file};
File dirFile = new File(System.getProperty("userdir"));
String[] envp = null;
if((!cmd[0].contains("*") && !cmd[0].contains("&")) &&
(!cmd[1].contains("*") && !cmd[1].contains("&")) &&
(!cmd[2].contains("*") && !cmd[2].contains("&")) &&
(!cmd[3].contains("*") && !cmd[3].contains("&")) &&
(!dirFile.getAbsolutePath().contains("*") && !dirFile.getAbsolutePath().contains("&")) &&
(envp == null)) {
Process pro = Runtime.getRuntime().exec(cmd, envp, dirFile);
But still getting the same OS Command Injection in the same line of process execution.
Can anyone help in resolving the issue.
.png){kind=logo}
Hi @SKumar P226205 (Community Member) ,
Veracode Static Analysis will report CWE 78 Improper Neutralization of Special Elements used in an OS Command (OS Command Injection) if it can detect that there is data from outside of the application (HTTP Request, File, Database, webservice, etc.) being used in a command or it's arguments.
We recommend hardcoding allowed inputs and using only hardcoded data in OS commands executed by the application.
If this is not possible and you must allow some dynamic inputs, then we recommend adding strict whitelist validation and proposing a mitigation ( https://help.veracode.com/reader/DGHxSJy3Gn3gtuSIN2jkRQ/~p4MSKOS8F8X8h0KwFTKoQ ) on this flaw from the Veracode Platform or IDE plugin describing the controls you have in place to prevent an attacker from abusing this functionality and contacting your security team to have these proposals reviewed.
The stated code does not appear to do whitelist validation, but rather blacklist validation by checking for "&". We recommend keeping a Positive Security Model ( https://www.owasp.org/index.php/Positive_security_model ) wherein you specify only a small number of allowed characters.
Unfortunately, it's impossible to give you a fixed routine or code snippet to use at what to use depends on the context of your application.
If you would like to review this in detail please feel free to contact the Veracode Application Security Consulting service either through Veracode Support (clicking your avatar in the upper right corner and then clicking "Contact Support" in the resulting menu) or through scheduling a consultation: https://help.veracode.com/reader/_xqy2rBkFBfaxzuiNu~wsg/kAO0UeevRvOPnidK6_e5IA .
Please let me know if you have any remaining questions or concerns.
Thank you,
Boy Baukema
Actually, Veracode recognizes a list of escaping methods and any attempts to write your solution manually is just rejected.
Thanks to https://stackoverflow.com/questions/65086887/crlf-injection-vulnerability-while-using-slf4j-logger-in-veracode-cwe-117 here is the list of allowed escaping methods: https://help.veracode.com/r/c_frameworks
Thus, change your code to
cmd[0] = java.net.URLEncoder.encode(cmd[0]);
...
Hi @popovr (Community Member) ,
You're absolutely correct that Veracode Static Analysis does not detect manual 'escaping'. You can find our supported cleansing functions here: https://help.veracode.com/r/review_cleansers .
Unfortunately, for this flaw category there is no supported cleansing function in Java. URLEncoder.encode is not supported nor recommended.
You must either hardcode the inputs (this can be automatically verified) or you must ensure the data is from a trusted source or appropriately whitelisted. Veracode Static Analysis is unable to verify this so you must then propose a mitigation and have this reviewed by your security team.
Thank you,
Boy Baukema
Hello, Boy,
Thank you for your response. Could you please provide reference to documentation that states that java.net.URL.encode is not recommended method to sanitize external input, that cannot be whitelisted nor blacklisted? Indeed, it does not escape or replace the <>|&"'` characters, but, this is the only one method of JSE Veracode accepts. What are the alternatives as Java standard methods?
Hi @popovr (Community Member) ,
You can find our supported cleansing functions per CWE at https://help.veracode.com/r/review_cleansers . Please note that for CWE 78 no functions are listed.
We have more general remediation advice at: https://www.veracode.com/security/dotnet/cwe-78 . However, please note that this cannot be automatically verified by Veracode Static Analysis and must be manually reviewed by submitting a mitigation proposal and reaching out to your security team for a review. You can find more documentation on the mitigation workflow here: https://help.veracode.com/r/improve_mitigation .
Thank you,
Boy Baukema
Hello @Boy, Security Consultant (Veracode),
I work with Java, so I checked https://www.veracode.com/security/java/cwe-78 as a description of the fix of CWE-78. This is exactly what I applied, namely calling Runtime.exec(String[]), but Veracode declared that as an error, because two of the parameters were recognized as coming from OS environment variables. It did not recognize neither pattern matching (whitelisting, blacklisting), nor character replacement, nor deletion, nor escaping of the &|"'><;() characters. The only thing Veracode recognized was applying the "cleansing" function, jaca.net.URLEncoder.encode from https://help.veracode.com/r/review_cleansers . I chose it as the only one available in JavaSE (all others come from specific 3rd party libraries or sun.* package). There was no need of that method as the exec(String[]) method does not call the shell and does not interpret the parameter. The only accepted code was:
public static boolean generateAlert(String system, String component, String text, int level) {
String message;
String jobset;
String jobname;
// If we have the jobset name, use it. If not, default to the system name
system = sanitize(system);
jobset = sanitize(System.getenv("JOBSET"));
if (jobset != null) {
system = jobset;
}
component = sanitize(component);
jobname = sanitize(System.getenv("JOBNAME"));
if (jobname != null) {
component = jobname+ "_"+ component;
}
message = system + ": EV_APP " + system + " " + component + " " + level + " " + text;
return generateAlert(message); // <<< This effectively calls Runtime.exec(String[])
}
private static String sanitize(String plainText) {
String result;
try {
result = java.net.URLEncoder.encode(plainText);
} catch (NullPointerException ex) {
result = null;
}
return result;
}
an no checks for null, no other means of sanitization were accepted, despite the recommendations in https://www.veracode.com/security/java/cwe-78
Veracode Static Analysis is not capable of automatically verifying all correct remediations for many flaws, including CWE 95 and will require mitigation. If you have questions on what's appropriate, I would recommend you schedule a consultation call to discuss.
You can check out this knowledge article (https://community.veracode.com/s/article/How-to-schedule-a-consultation-call) on how to schedule a consultation call with us.
I'm very sorry to hear that applying URLEncode closes the flaw, it should not. I would not recommend relying on this behaviour as we will likely correct it at some point.
Could you please contact support and provide them with the details for a scan wherein URLEncoder closed the CWE 95 flaw and link them to this post?
Here's how you can log a case:
1. Navigate to the upper right corner of any page in the Community, click on your user avatar.
2. Select Contact Support from the drop-down menu.
Thank you,
Boy Baukema