Svajjarapu023153 (Community Member) asked a question.
Here is the details of code snippet which have done to resolve the OS Command injection in my code base.
Before:
process = new ProcessBuilder(argList).directory(new File(userDir)).redirectErrorStream(true).start();
final BufferedReader br = new BufferedReader(new InputStreamReader(process.getInputStream()));
After:
final Pattern whitelist = Pattern.compile("*^[a-zA-Z0-9.]*$");
Process process=null;
if(whitelist.matcher(String.valueOf(argList.get(0))).matches()) {
process = new ProcessBuilder(argList).directory(new File(userDir)).redirectErrorStream(true).start();
}
final BufferedReader br = new BufferedReader(new InputStreamReader(process.getInputStream()));
Still Veracode reports these changes as flaw. Any recommendation will be appreciable.
.png){kind=logo}
Hi @Svajjarapu023153 (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 (as you have done for the first element, though not for any others arguments) 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.
Please let me know if you have any remaining questions or concerns.
Thank you,
Boy Baukema
Hi Boy,
Thanks for your response. Flaw is not rectified even whitelist validation applied for all arguments and mitigated this flaw in Veracode platform.
Thanks,
Sony.
Hi Sony,
In the provided scenario, Veracode identified a "Flaw", requiring attention. You were able to improve your code, with a very good solution. (Although, i'm not entirely sure what the first '*' in the regex is supposed to do). The downside is that Veracode didn't recognize it automatically after rescan. Your solution is evidence that the code has been improved and is more secure. Personally, I would consider the identified flaw as mitigated, and would go thru the mitigation process to remove it from future reports. The link Boy provided is to the documentation to the mitigation and approval process.
I suppose ... Happy the tool provided a magnifying glass to code that could be improved. OK to be bummed for needing to take an extra step to remove it from the report!
Hi @DeCaPa (Community Member) ,
Yes, a whitelist solution will typically mitigate the risk and should be documented with a mitigation proposal which must then be communicated to and reviewed by the security team
Unfortunately, there are no standard controls that we are able to automatically recognize (if you want to go down the rabbit hole I recommend the following starting point: https://stackoverflow.com/a/4094897/4512 ) .
Perhaps something like "StringUtils.isAlphanumeric" might be something we could support?
Or perhaps a Custom Cleanser ( https://help.veracode.com/reader/DGHxSJy3Gn3gtuSIN2jkRQ/xrEjru~XmUHpO6~0FSae2Q ) would be sufficient?
If you have any ideas on standard controls that we can recognize I would very much like to request submitting them to https://community.veracode.com/s/ideas for consideration.
Thank you,
Boy Baukema