Skip to main content

Three Apex details security scanners get wrong

· 8 min read
Vulkro
Vulkro
Security research

Apex looks enough like Java that it is easy to read it as Java. Security tooling does the same, and the places where Apex is not Java are where findings go missing. A scanner that matches Database.query( will not see DataBase.Query(. A reviewer who sees an access check at the top of a method tends to assume everything below it is covered.

This post walks through three such details. Each one hid real findings in our own engine until vulkro-sf 0.27.0, and each one is just as easy to miss in a manual review. For every case there is vulnerable code, the reason it is vulnerable, and a fixed version.

1. Apex ignores case, and comments are not code​

The Apex Developer Guide is explicit that Apex is case-insensitive, like SOQL and SOSL: variable and method names, object and field references and query keywords all match regardless of capitalisation. Most developers write Database.query because the documentation does. The compiler accepts anything.

public with sharing class AccountSearch {
    @AuraEnabled
    public static List<Account> find(String name) {
        String q = 'SELECT Id, Name FROM Account WHERE Name = \'' + name + '\'';
        return DataBase.Query(q);
    }
}

This is a textbook SOQL injection. A rule written for Database.query( passes it as clean. The same is true for DML: Update acc; is an update statement, and a matcher looking for the lowercase keyword does not see it.

Case cuts the other way too. String.ESCAPESINGLEQUOTES(name) is the platform's escaping method. A tool that only recognises the canonical spelling will not credit it, and will report a query that is actually escaped.

Comments are the mirror problem. // insert the record after validation contains the word insert, and a tool that searches text rather than code will treat it as a DML statement. That produces findings on lines that do nothing.

The fix for the query itself is a static query with a bind variable, which leaves nothing to inject:

public with sharing class AccountSearch {
    @AuraEnabled
    public static List<Account> find(String name) {
        return [SELECT Id, Name FROM Account WHERE Name = :name WITH USER_MODE];
    }
}

2. An access check guards one operation, not the whole path​

Apex database operations historically run in system mode: they do not check the running user's object and field permissions unless the code asks for it. The code can ask in several ways: a describe check such as isUpdateable(), WITH USER_MODE on a query, as user on a DML statement, or Security.stripInaccessible on the records. Each one protects the operation it is attached to. None of them protects the method.

public with sharing class ClaimService {
    @AuraEnabled
    public static void attachDocument(Id claimId, String fileName) {
        if (!Schema.sObjectType.Claim__c.isUpdateable()) {
            throw new AuraHandledException('Not allowed');
        }
        Claim__c c = [SELECT Id, Status__c FROM Claim__c
                      WHERE Id = :claimId WITH USER_MODE];
        c.Status__c = 'Documents received';
        update c;

        insert new Claim_Document__c(Claim__c = claimId, Name = fileName);
    }
}

Read top to bottom, this method looks careful. There is a permission check and a user-mode query. But look at what each one covers:

  • isUpdateable() checks object-level edit access on Claim__c. It says nothing about the Status__c field and nothing about Claim_Document__c.
  • WITH USER_MODE applies to the query it sits in. The update two lines later runs in the class's default mode.
  • The insert of Claim_Document__c has no check at all. A user who cannot create claim documents through the UI can create them by calling this method.

A tool that clears the whole method because it saw an access check somewhere on the path will report nothing here. We did exactly that before 0.27.0.

The fix gives each operation its own enforcement:

public with sharing class ClaimService {
    @AuraEnabled
    public static void attachDocument(Id claimId, String fileName) {
        Claim__c c = [SELECT Id, Status__c FROM Claim__c
                      WHERE Id = :claimId WITH USER_MODE];
        c.Status__c = 'Documents received';
        update as user c;

        insert as user new Claim_Document__c(Claim__c = claimId, Name = fileName);
    }
}

User-mode DML enforces object permissions, field permissions and sharing for that statement. Where you would rather drop fields the user cannot write than fail, Security.stripInaccessible(AccessType.CREATABLE, records) followed by an insert of the returned records does the same job, as long as you insert what it returns and not the original list.

The Apex Developer Guide notes that for code saved at API version 67.0 or later, user mode is the default. Code saved at older versions keeps the older behaviour, and most orgs have plenty of it, so it is worth checking the version on any class that writes data.

3. Escaping one piece does not make the query safe​

String.escapeSingleQuotes is the platform's escape hatch for dynamic SOQL. Salesforce's own SOQL injection guidance prefers static queries with bind variables, and offers escaping for the cases where a dynamic query cannot be avoided. Escaping protects one value, inside quotes. Dynamic queries are rarely one value.

public with sharing class CaseSearch {
    @AuraEnabled
    public static List<Case> search(String subject, String sortField) {
        String q = 'SELECT Id, Subject FROM Case WHERE Subject LIKE \'%'
            + String.escapeSingleQuotes(subject) + '%\''
            + ' ORDER BY ' + sortField;
        return Database.query(q);
    }
}

The subject value is escaped and sits inside quotes, so it is fine. The sortField value is appended raw. Anyone who can call the method controls the end of the query. A tool that sees escapeSingleQuotes in the expression and marks the whole string as sanitised misses it.

Two related traps follow the same logic:

  • An escaped value outside quotes protects nothing. '... WHERE Amount > ' + String.escapeSingleQuotes(minAmount) has no quote to break out of, so escaping has no effect and the value is still injectable.
  • Lists hide the raw value. Code that builds a list of conditions, escapes some of the values it adds and then joins the list into the query has the same problem as the string above: one escaped entry does not clean the others.

The fix is to bind the values and allowlist anything that cannot be bound, such as a field name:

public with sharing class CaseSearch {
    private static final Set<String> SORTABLE =
        new Set<String>{ 'Subject', 'CreatedDate' };

    @AuraEnabled
    public static List<Case> search(String subject, String sortField) {
        if (!SORTABLE.contains(sortField)) {
            sortField = 'CreatedDate';
        }
        Map<String, Object> binds = new Map<String, Object>{
            'pattern' => '%' + subject + '%'
        };
        return Database.queryWithBinds(
            'SELECT Id, Subject FROM Case WHERE Subject LIKE :pattern ORDER BY ' + sortField,
            binds,
            AccessLevel.USER_MODE
        );
    }
}

The search value is now a bind variable, the sort field can only be one of two known names, and the query runs in user mode.

What changed in vulkro-sf 0.27.0​

All three are fixes to the Apex engine that vulkro-sf scan runs:

  • Apex calls are found whatever their capitalisation. DataBase.Query(q), Update acc; and String.ESCAPESINGLEQUOTES(x) are recognised as the platform calls they are, so the first example is reported as SOQL injection (apex-taint-soql-injection) and an escape in capitals is credited as an escape. A comment that mentions a keyword no longer counts as code.
  • Access checks protect only the operation they guard. A CRUD or field-level security check on one object, or a WITH USER_MODE read, no longer clears an unguarded insert or update of another object on the same path. In the second example the Claim_Document__c insert is reported (apex-taint-crud-fls), and the finding names the unprotected operation rather than the first data access on the path.
  • An escaped value no longer hides a raw one. In a dynamic query built from several pieces, escaping one piece no longer marks the whole query as safe, and the same holds for values added to a list. The sortField in the third example is reported. An escaped value placed outside quotes has its own check, escape-single-quotes-call.

All three find bugs we used to miss. The case and comment handling also removes findings that should never have been reported, such as an escaped query whose escape was spelled in capitals. We would rather tell you both than present a recall gain on its own.

One limit worth stating: these are source checks. They tell you that an operation runs without enforcement and whether untrusted input reaches it. Whether that matters in your org depends on who can call the class, which is where profile and permission set metadata come in. When a guest profile can call the method, the finding becomes part of the guest attack path we walked through in the anatomy of a guest user data leak.

Where to see it​

Run vulkro-sf scan ./force-app on your project, or install the VS Code extension to see these findings on the line while you write Apex. The Apex detectors page lists what the engine checks, and the scan reference covers the flags. More on Vulkro for Salesforce is on the Salesforce page. To get set up, start here.

Sources​