phoedos/pmd
1
1
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Added information about an observed bug with authorization checks against sub-relations in static SOQL queries. Added a test case that reproduces the bug. Once the bug is fixed, the unit test expectation can be changed to provide both verification and a check against regressions.

Browse Source
This commit is contained in:
Scott Wells 2021-10-21 16:47:16 -05:00
parent baad5701dc
commit 873f3527b2
2 changed files with 31 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexCRUDViolationRule.java
View File

@ -660,6 +660,9 @@ public class ApexCRUDViolationRule extends AbstractApexRule {
}
private void checkForAccessibility(final ASTSoqlExpression node, Object data) {
// TODO: This includes sub-relation queries which are incorrectly flagged because you authorize the type
// and not the sub-relation name. Should we (optionally) exclude sub-relations until/unless they can be
// resolved to the proper SObject type?
final Set<String> typesFromSOQL = getTypesFromSOQLQuery(node);
final Set<ASTMethodCallExpression> prevCalls = getPreviousMethodCalls(node);

28
pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexCRUDViolation.xml
View File

@ -1489,6 +1489,34 @@ public class Foo {
merge masterAccount mergeAccount;
}
}
}
]]></code>
</test-code>
<!-- Demonstrate that authorization for sub-relation queries doesn't work properly; see checkForAccessibility() -->
<test-code>
<description>Demonstrate that authorization for sub-relation queries doesn't work properly</description>
<!-- This is what we should get if things were working properly -->
<!--<expected-problems>0</expected-problems>-->
<!-- But this is what we actually get -->
<expected-problems>1</expected-problems>
<code><![CDATA[
public class Foo {
public void foo() {
if (Account.SObjectType.getDescribe().isAccessible() &&
// This check isn't working because it's checking against "Contacts" due to the nested FROM clause
// instead of "Contact" given that "Contacts" is of type "List<Contact>"
Contact.SObjectType.getDescribe().isAccessible()) {
List<Account> accounts = [
SELECT Id, (
SELECT Id
FROM Contacts
)
FROM Account
];
}
}
}
]]></code>
</test-code>