4

In security scan am getting error as "SOQL Injection Vulnerability" and also the note as "The query is user controllable"

string query = 'SELECT '+ queryFields +' From '+MFNameSpaceUtil.PrependNS('MobiForm_Field__c')+' WHERE ';
    return query;

This is my query and doesn't have any idea what would cause the SOQL injection here. Any suggestions will be helpful

  • 1
    This appears to be the same kind of vulnerability as a SQL injection vulnerability. You should read up on them. If queryFields comes from any user controlled input (even a web request from a browser), then a malicious user could craft it to perform arbitrary operations on the DB. – jpmc26 Jan 30 '18 at 9:35
  • 1
    That is not a valid query. You will never be able to run SELECT ... FROM ... WHERE since your query cannot end with that keyword. – Adrian Larson Jan 30 '18 at 17:57
  • 1
    @jpmc26 No nothing is from user input, queryFields are all fields in that object and calling this method and returning the formed query , thanks – SFDC Jan 31 '18 at 5:06
  • @AdrianLarson no am just calling this method and returning the formed query, and later adding the wherecondtion with this query , thanks – SFDC Jan 31 '18 at 5:07
  • 1
    @jpmc26 "MobiForm_Field__c"--?MFNameSpaceUtil.PrependNS('MobiForm_Field__c') is for namespace. A doubt came for me is this because hardCoding of ObjectName(above) - should I assign it to a string and pass it ?? – SFDC Jan 31 '18 at 6:20
9

Long Story:

Quoting examples from apex documentation:

// User supplied value: name = Bob 
// Query string
SELECT Id FROM Contact WHERE (IsDeleted = false and Name like '%Bob%')


However, what if the user provides unexpected input, such as:

// User supplied value for name: test%') OR (Name LIKE '

SELECT Id FROM Contact WHERE (IsDeleted = false AND Name LIKE '%test%') OR (Name LIKE '%')

To fix this you can either use static SOQL and re-write your SOQL as below

To prevent a SOQL injection attack, avoid using dynamic SOQL queries. Instead, use static queries and binding variables. The vulnerable example above can be re-written using static SOQL as follows:

public class SOQLController { 
    public String name { 
        get { return name;} 
        set { name = value;} 
    } 
    public PageReference query() { 
        String queryName = '%' + name + '%';
        queryResult = [SELECT Id FROM Contact WHERE 
           (IsDeleted = false and Name like :queryName)];
        return null; 
    } 
} 

For dynamic SOQL use String strEsc = String.escapeSingleQuotes(str);//str is your searchspec from UI

This method adds the escape character () to all single quotation marks in a string that is passed in from a user. The method ensures that all single quotation marks are treated as enclosing strings, instead of database commands.

For your use case:

You seem to be constructing the SOQL dynamically. your issue could be with your where clause if the bind variable for it comes from UI.

  • so you are saying nothing is wrong with the above code, and it might be from the where clause below ?? Is there any specific place where I should use String strEsc = String.escapeSingleQuotes(str); can use for all variables in dynamic SOQL right ?? – SFDC Feb 9 '18 at 12:59
6

One further observation is that dynamic SOQL does support simple (not dotted expressions or method calls) bind variables that also protect against SOQL injection.

So for some dynamic SOQL cases you can use do something like this:

 String nameLike = '%' + nameSearchField +'%';

 String query = 'select ... from ... where Name like :nameLike';

that is a little cleaner than concatenating in String.escapeSingleQuotes calls.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.