7

I have Following page:

<apex:page controller="myclass"  action={!clean}>
</apex:page>

and controller class as:
public with sharing class myclass
{
public void clean()
{
    List<auditcount__c> existing = [SELECT Id From auditcount__c LIMIT 10];
    delete existing;
}
}

When I try scanning my code for the security review, I get Cross Site Reference Forgery error for the above page. Can anyone please help on how should I enforce Cross Site Reference Forgery in the above VF page? I saw the examples salesforce gave but i couldn't apply it to my case as I need to delete all record from object.

3

I've tried to achieve something similar with: CSRF safe Custom button linked to Apex method

I believe you will need to execute clean from a command button or similar after the page has loaded. This will pickup the ViewStateCSRF protection that Salesforce builds into forms to verify that the user actually pressed the button and posted back.

As your page is now, I could put an iframe in my website that would cause your browser to send a GET request and delete those items without any interaction from you (assuming you were logged into Salesforce at the time).

3

The underlying problem here is that you are executing DML to delete data from a GET request. Thus the user has no way to confirm that the deletion is correct. While this might seem like a slick solution to a particular problem, it is a security hole.

Imagine there is another site that the user accesses on a regular basis (which might be an internal one), and this has links that the user clicks without thinking. A simple tweak to the web page can cause it to output a link to your page passing a randomly generated record id, or possibly one of a list of real record ids. If you have an active Salesforce session, the record would be deleted upon clicking the link. With a little creativity and some javascript, this could be set up to delete a number of records from a single click.

The only safe way to avoid this is not to carry out any DML unless the user confirmed it - i.e. from action methods that are called from command buttons/links in response to the user pressing them.

As you are going through the security scanner and security review, I'd imagine that means you are attempting to get a managed package onto the app exchange. You won't get through with this kind of vulnerability I'm afraid.

2

Honestly, this shouldn't be something you need to worry about, but the way the apex:page action attribute is architected it results in immediate processing via a GET request. This means if a malicious hacker put an image like <img src="http://na1.salesforce.com/apex/Clean"/> in there website and the user had an active session on the na1 instance it would be cause your clean function to run potentially wiping out data!!. In contrast any actions done on the page are via a POST request which includes a CSRF token that is used to distinguish actions that come from bad places.

You can work around this by having an onload script run your action (which seems like how they should update the apex:page action attribute to work in the first place instead of counting on people to actively avoid using a native feature with a security hole).

Example

<apex:page controller="MyClass">
Processing...

<!-- Submit the page via action function on onload event to avoid XSRF issues -->
<apex:includeScript value="//ajax.googleapis.com/ajax/libs/jquery/2.0.0/jquery.min.js"/>
<apex:form >
    <apex:actionFunction name="clean" action="{!clean}"/>
    <script>
        j$ = jQuery.noConflict();
        j$().ready(function() {
            clean();
        });
    </script>
</apex:form>

</apex:page>
  • 1
    I'd argue that using actionFunction to invoke the method has only hidden the CSRF vulnerability from the apex security scanner. It hasn't addressed the CSRF security issue. – Daniel Ballinger Apr 23 '13 at 2:53
  • 2
    Exactly - this might get you past the security scanner but the security review will definitely pick up on it. – Bob Buzzard Apr 23 '13 at 7:55
  • @DanielBallinger I guess I'm misunderstanding the vulnerability, won't the action function be a POST and include the CSRF token? – Ralph Callaway Apr 23 '13 at 14:12
  • @BobBuzzard that probably should be the case if I'm just creating another security hole, but in past security reviews this hasn't come up. – Ralph Callaway Apr 23 '13 at 14:15
  • 1
    The vulnerability is still there. You have the token as it is a post, but as you have automated calling the clean function, a malicious site could still cause records to be deleted. The net effect is the same as executing the clean method as the page action. – Bob Buzzard Apr 23 '13 at 16:34
1

The scanner simply reports these when changes are made to data inside a constructor. If you can show that the changes made aren't an issue even if the request is illegitimate then you consider it a false positive.

In this case however, you're deleting data, so having a confirmation built into this page would be the best option here.

  • I'm guessing that the user already clicked a button to get to this page, and confirming again isn't exactly the best user experience. Anyway to avoid having to double up on the confirmations? – Ralph Callaway Apr 23 '13 at 14:17
  • 1
    The problem is that you can't guarantee how the user got to the page - hence the vulnerability. – Bob Buzzard Apr 23 '13 at 16:35
  • There's pretty much nothing you can do against that though, the only thing you can do is just make sure somebody is at least interacting with the page. The scanner flags the issue because no interaction is required. You hit the page, and data gets modified (deleted in this case). – Matt Lacey Apr 24 '13 at 1:09

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.