Spot The Bug #2

The first Spot The Bug garnered about 40 comments.  I've updated the initial prize/rule post a bit to better reflect the reality of folks answering from all over the world.  What potential security issues exist in the code below?  Please post answers below in comments.  I'm looking for quality over speed 🙂

Visualforce

<apex:page controller="searchAccounts">
  <apex:pageBlock title="Find Accounts">
    <apex:form >
      <apex:outputText value
=" Enter Zip Code    " ></apex:outputText>
      <apex:inputText value="{!zipCode}" />
      <apex:commandButton value="Query" action="{!query}" ></apex:commandButton>  
    </apex:form>
  </apex:pageBlock>
  <apex:pageBlock title=
"Accounts">
    <apex:dataList value="{!queryResult}" var="r">
      <apex:outputLink value="http://maps.google.com/maps?f=d&source=s_d&saddr=&daddr={!r.BillingStreet},{!r.BillingCity},{!r.BillingState},{!r.BillingPostalCode}" target="_blank">{!r.Name}</apex:outputLink>
    </apex:dataList>
  </apex:pageBlock>
 </apex:page>

Apex

public class searchAccounts { 
  public String zipCode { get; set; }
  public List<Account> queryResult { get;  set; } 

  public PageReference query() {
    String zipFieldName ;             
    // create the query string 
    String qryString = 'SELECT Name, BillingStreet, BillingCity, BillingState, BillingPostalCode FROM Account WHERE BillingPostalCode = ' + ''' + cleanse(zipCode) + ''' ; 
    // execute the query
    queryResult = Database.query(qryString) ;       
    return null;
  } 

  public String cleanse(String a)
  {
    String clean;
    clean = a.replace(''', ''');
    return clean;
  }
 }

 


Update

There were several solid answers.  E.J. nailed it early on with his two answers:

  1. Sharing was not enforced in the code.  The 'with sharing' keyword should have been used.
  2. The cleanse function, while a good attempt at removing single quotes and preventing SOQL Injection, did not escape the character.  An attacker could send ' as part of the input which would go through the cleanse function as '.  Effectively ensuring only the slash was escaped and not the single quote.  The proper solution is to use bind variables wherever possible, but String.escapeSingleQuotes could also be used.

 

Here's some updated Apex code.

Apex

public with sharing class searchAccounts { 
  …

  public PageReference query() {
    String zipFieldName ;            
    //use bind variable
    queryResult = [SELECT Name, BillingStreet, BillingCity, BillingState, BillingPostalCode FROM Account WHERE BillingPostalCode =:zipCode];    
  
    return null;

  } 
 }


Good Luck!

 -Robert

Leave your comments...

Spot The Bug #2