RHarper142384 (Community Member) asked a question.
I am developing code for an Adobe AEM instance. I am trying to figure out how to fix a CWE-90 issue.
I have a couple queries like the following that are being called out. I've validated all of the variables used but I keep having this line flagged. The ResourceResolver does not have a parameterized query like JDBC does. The query doesn't support an update, it is query only. In truth, AEM checks the permissions on resources and even though they might qualify in general, the user will not see them if they do not have access rights.
Iterator<Resource> files = resourceResolver.findResources( "SELECT * FROM [nt:file] AS s WHERE ISDESCENDANTNODE([/content]) and CONTAINS(s.*, '" + fragment + "')", "JCR-SQL2" );
What do I need to change to get this to pass?
.png){kind=logo}
Hi @RHarper142384 (Community Member) ,
Veracode Static Analysis reports flaws of CWE 99 Resource Injection in Adobe Experience Manager (Apache Sling) binaries . by looking at looking at queries done with the ResourceResolver (for example with findResources or getResource) and determining if any part of the query contains data from outside of the application (for example from HTTP Request, but also from database, files, etc.).
If we can find such a path we will report this for the potential for Resource Injection. The concern being that an attacker may be able to alter the query to access resources they should not have access to. While the Java Content Repository (JCR) may block access we don't recommend relying on correct configuration of all resources.
Instead we recommend either ensuring the input to the query is hardcoded (which will close the flaw) or you put some validation in place (in which case you should propose a mitigation and contact your security team to review it, see also: https://help.veracode.com/reader/DGHxSJy3Gn3gtuSIN2jkRQ/~p4MSKOS8F8X8h0KwFTKoQ ).
Thank you,
Boy Baukema
The problem I have is that one of the failing queries does not use any data from an outside source. It is querying based on a path of another resource. Even when I scrub the supplied information to ensure that there aren't any characters other than identification characters and numbers an no possibility of having path information or expected path information, it still complains. I don't see the difference of using a query string and building the same query with parameters. I have it passing now but there is more overhead because I have to use the QueryBuilder to create the same query.
A resource, originating from the JCR, to Veracode Static Analysis, is an outside source of data.
Veracode Static Analysis does not consider the JCR data (or any data from outside of the supplied binary) 'trusted'.
Veracode Static Analysis does not automatically evaluate custom validation logic, if you're using custom logic to clean the data you must propose a mitigation and have this reviewed by your security team.
The QueryBuilder API ( https://helpx.adobe.com/experience-manager/6-3/sites/developing/using/querybuilder-api.html ) does not do any validation (nor can it as it very much depends on your use case) and I'm sorry to hear that we're unable to see that it propagates possibly attacker controlled data. I have asked our Research team to investigate this and I would not recommend relying on this behaviour, rather proposing a mitigation documenting the core question "What controls do you have in place to prevent an attacker from querying arbitrary resources?" and having this proposal reviewed by your security team.
If you have any other questions, 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.
Thank you,
Boy Baukema
Okay. So I'm even more confused on what you are expecting to see for validating and scrubbing the data. Your documentation also recommends using parameterized SQL queries. If you are saying that using the query builder to create the query is not acceptable, then why is a parameterized SQL query? Seems to be the same thing.
I'm now assuming you are expecting to see some form of input validation by using some class(es) to do that work. What patter/classes are you expecting to see when scrubbing data? I have my code passing now but I want to get this right going forward.
Hi @RHarper142384 (Community Member) ,
Using a QueryBuilder is very much a good pattern as it prevents an attacker from being able to inject into the query.
For example if I were to do:
// WARNING DANGEROUS CODE DO NOT USE
URL url = new URL("http://localhost:4502/bin/querybuilder.json?path=" + request.getParameter("path"));
HttpURLConnection servletConnection = (HttpURLConnection) url.openConnection();
servletConnection.setRequestMethod("GET");
servletConnection.setDoOutput(true);
InputStream response = servletConnection.getInputStream();
// WARNING DANGEROUS CODE DO NOT USE
I would not only have control over the path, but also over any other argument in the URL by just adding &p.limit=10000 for example.
The QueryBuilder prevents that.
However, it does nothing to prevent an attacker from being able to reference an arbitrary path, for example the following would still allow them to specify an arbitrary path:
// WARNING DANGEROUS CODE DO NOT USE
Map<String, String> map = new HashMap<String, String>();
map.put("path", request.getParameter("path"));
Query query = builder.createQuery(PredicateGroup.create(map), session);
SearchResult result = query.getResult();
// WARNING DANGEROUS CODE DO NOT USE
The path should be either hardcoded, or validated as strictly as possible, like so:
Pattern pattern = Pattern.compile("^/books/\\w{0,255}$");
Matcher matcher = pattern.matcher(path);
if (!matcher.find()){
throw new SecurityException("Invalid path specified in request");
}
Map<String, String> map = new HashMap<String, String>();
map.put("path", path);
Query query = builder.createQuery(PredicateGroup.create(map), session);
SearchResult result = query.getResult();
Note that what paths are and are not safe to query depends very much on the functionality. In this example this is allowed to search books and only books. Your example will likely be more complex.
As this depends on the semantics of your application Veracode Static Analysis is unable to automatically detect this and you must then propose a mitigation describing the validation you have implemented.
If you have any other questions, 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.
Thank you,
Boy Baukema