Spot The Bug #3

Here's a new Spot The Bug.  It might be a little trickier than the others, but we're looking for a specific security flaw in this code.  We're still giving away $50 gift certificates per this post.  Last time we had lots of high quality answers and quality still matters over quickness.  I'll give out the answer in a few days.

 

Visualforce

<apex:page controller="setupTransfer">
    <apex:pageBlock title="Setup Money Transfer">
        <apex:form >
            <apex:outputLabel >From Account :&nbsp;</apex:outputLabel><apex:inputText value="{!mFrom}" ></apex:inputText><br/>
            <apex:outputLabel >To Account :&nbsp;</apex:outputLabel><apex:inputText value="{!mTo}" ></apex:inputText><br/>
            <apex:outputLabel >Amount :&nbsp;</apex:outputLabel><apex:inputText value="{!mAmount}" ></apex:inputText><br/>
            <apex:commandButton value="Setup Transfer" action="{!doSetupTransfer}" ></apex:commandButton> 
        </apex:form>
    </apex:pageBlock>
</apex:page>

 

Apex

public with sharing class setupTransfer {

    public String mFrom {get; set;}
    public String mTo {get; set;}
    public String mAmount {get; set;}

    public PageReference doSetupTransfer() {
        Transfer__c transferRecord = new Transfer__c();
        transferRecord.FromAccount__c = mFrom;
        transferRecord.ToAccount__c = mTo;
        transferRecord.Amount__c = Decimal.valueOf(mAmount);

        // Insert the new transfer record into DB
        insert transferRecord;

        // Redirect the user to the detail page for the new transfer record.
        PageReference recDetailsPage = new ApexPages.StandardController(transferRecord).view();
        recDetailsPage.setRedirect(true);
        return recDetailsPage;
    }
}

 

Update


Gareth nailed what we were looking for (Gareth please leave a comment with your email address in the optional email address textbox and we'll get you your prize).  The code is not checking CRUD/FLS appropriately which could lead to users without the appropriate privilege abusing this.

There are a couple ways to solve this.  First, inputField could be used which will leverage the existing security measures built into the platform.  Second, the Apex code can check CRUD and FLS itself.  Below you'll find a reference to the open source Force.com ESAPI library which can be used to perform this and other security checks.

Apex

public with sharing class setupTransfer {
    public String mFrom {get; set;}
    public String mTo {get; set;}
    public String mAmount {get; set;}

    public PageReference doSetupTransfer() {
        Transfer__c transferRecord = new Transfer__c();

        try {
            transferRecord.FromAccount__c = mFrom;
            transferRecord.ToAccount__c = mTo;
            transferRecord.Amount__c = Decimal.valueOf(mAmount); 

            List<Schema.SObjectField> fieldsToInsert = new List<Schema.SObjectField> {
                    Transfer__c.FromAccount__c,
                    Transfer__c.ToAccount__c,
                    Transfer__c.Amount__c
                }; 

            transferRecord = (Transfer__c)ESAPI.accessController().insertAsUser(transferRecord, fieldsToInsert);

        } catch (SFDCAccessControlException e) {
            // error handling
            return null;
        }

        // Redirect the user to the detail page for the new transfer record.
        PageReference recDetailsPage = new ApexPages.StandardController(transferRecord).view();
        recDetailsPage.setRedirect(true);
        return recDetailsPage;

    }
}

 

Good luck!
   -Robert 

Leave your comments...

Spot The Bug #3