0

We are using a third party company to scan our salesforce code for SOQL Injection and Vulnerability issues with open page redirects. The below code is flagged as a vulnerability.

    Id id = ApexPages.currentPage().getParameters().get('id');
    String html;                                                                        
    **if (Utilities.isId(id) && !String.valueOf(id).startsWith('/')) {**                
        html = new PageReference('/' + id + '/p').getContent().toString();
    } else {
        html = new PageReference('/' + '/p').getContent().toString();                   
    }                                                                                   

I am checking whether the variable id is an instance of Salesforce Id using the Utilities.isId method and also checking to see there is no forward slash at the beginning and I am adding the forward slash while constructing the html String variable. However, this fix is still flagged as vulnerable code.

Can someone help?

The implementation of isId method is below.

public boolean isId(String strId) {
    return strId instance Id;
}
0

It sounds like a false positive, assuming Utilities.isId is safe. There's really not much of a need for the complicated check, though, as the following would suffice:

String recordId = ApexPages.currentPage().getParameters().get('id');
if(recordId instanceOf Id) {
  ...

An ID can never start with a /, so if this passes, it's an ID.

| improve this answer | |
  • implementation of isId method is added to the original post. – Balaji Pooruli May 27 at 18:00
  • @BalajiPooruli Which is basically what I wrote, except it really doesn't need to be a method, if you ask me. Still, yes, this should be perfectly safe. – sfdcfox May 27 at 18:03

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.