4

I am facing following security review issue.

I am getting SOQL/SOSL security review issue for the below format of code:

String whereclause -> name = 'test'; //assume whereclause entered by user

String str = 'select id from account where' + whereclause; 

System.debug(str); //---> select id from account where name = 'test'

Database.query(strEsc);  

I have tried to solve this issue using String.escapesingleQuote method but I am getting

no viable alternative at character '\' error.

Please see the code below:

String whereclause -> name = 'test'; //assume whereclause entered by user

String str = 'select id from account where' + whereclause; 

System.debug(str); //---> select id from account where name = 'test'

String strEsc = String.escapeSingleQuotes(str);  //-->select id from account where name = \'test\'

Database.query(strEsc); //Error ( no viable alternative at character '\')

It's not a code issue as much as it is a design issue.

Yes @martin, you are correct.

To handle SOQL injections we have changed design something like this: enter image description here In our page, we are also planning to display filter criteria as it is like this. I will create a UI in such away that user able to choose Field, Operator and Value. In code level will use these variables escapeSingleQuotes method.

My question here is, Is this design will resolve SOQL injection issue?

Thanks

7
  • What part of the where clause is supplied by the user? Is it the whole name = 'test' or is it just test
    – martin
    Aug 23 '16 at 7:20
  • It is whole name = 'test'
    – Riya
    Aug 23 '16 at 7:21
  • The problem is that there is a conflict between the functionality that you want to provide and the type of actions that the soql-injection security review is trying to prevent. Allowing arbitrary text in the whereclause means that you want to allow soql-injectons.
    – martin
    Aug 23 '16 at 7:23
  • Yes, I understand that. Please let me know if any solution is there
    – Riya
    Aug 23 '16 at 7:25
  • It's not a code issue as much as it is a design issue.
    – martin
    Aug 23 '16 at 7:26
4

Here is an example. You will have to pretty it up but it shows how entering:

Name = 'test'

into the where clause causes errors with the escaped query but not the unescaped

Controller

public class SOQLInjectionTest{


    public string whereClause{
        get{
            return whereClause;
        }
        set;
    }

    public Account[] accts {get;set;}

    public string queryStringBasic{

       get{
           return 'Select ID From Account Where ' + whereClause;
       }
       set;

    }

    public string queryStringEscaped{

       get{
           return 'Select ID From Account Where ' + string.escapeSingleQuotes(whereClause);
       }
       set;

   }

   public void basicQuery(){
       accts = database.query(queryStringBasic); 

   }

   public void escapedQuery(){
       accts = database.query(queryStringEscaped); 
   }


}

VF Page (Needs prettied up but you get the point)

<apex:page controller="SOQLInjectionTest">


<apex:pageMessages id="msgs"/>


<apex:form >
    Enter Where Clause <apex:inputText value="{!whereClause}"/>
    <apex:commandButton value="No Escape Query" action="{!basicQuery}" rerender="msgs,opt_pnl"/>
    <apex:commandButton value="Escape Query" action="{!escapedQuery}" rerender="msgs,opt_pnl"/>


    <apex:outPutpanel id="opt_pnl" layout="block">
        Account: {!accts}
        Where Clause: {!whereClause}
    </apex:outPutPanel>
</apex:form>



</apex:page>

So given this how are we supposed to prevent injection. Or is this use case just a great example of what we should NOT be doing?

The examples provided here https://developer.salesforce.com/docs/atlas.en-us.pages.meta/pages/pages_security_tips_soql_injection.htm as a way to test injection results with injection: `test%') OR (Name LIKE '.

So does it depend on how the query is structured? It would seem that there is not a need to escape when you are constructing an entire where clause....but then you are leaving the query wide open anyway.....hmmmmm

So I guess you are left with trying to justify it and ensure you are only returning records the user has access to. `

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.