www.webdeveloper.com
Results 1 to 3 of 3

Thread: Is this safe from SQL injection?

  1. #1
    Join Date
    Nov 2013
    Posts
    4

    Is this safe from SQL injection?

    //GET PARAMS
    Code:
    if (isset($_GET['filter'])) {$filter = $_GET['filter'];} else {$filter = "Customer";}
    if (isset($_GET['terms'])) {$terms = $_GET['terms'];} else {$terms = null;}
    //WHITELIST
    Code:
    $filter_whitelist = $sort_whitelist = array("Customer", "Web", "Tax_ID");
    $dir_whitelist = array("asc","desc");
    //ENFORCE WHITELIST
    Code:
    if(!in_array($filter, $filter_whitelist)) {die("Invalid filter type.");}
    if(!in_array($sort, $sort_whitelist)) {die("Invalid sort type.");}
    if(!in_array($d, $dir_whitelist)) {die("Invalid direction.");}
    //QUERY
    Code:
    $stmt = $db->prepare("SELECT customers.ID, customers.Customer, customers.Size, customers.Status, customers.Web, customers.Contact_Email, resellers.Reseller, distributors.Distributor FROM customers INNER JOIN resellers on customers.Reseller=resellers.ID INNER JOIN distributors on resellers.Distributor=distributors.ID WHERE customers.Status='Prospect' AND $filter LIKE :terms ORDER BY $sort $d LIMIT :page, 18");
    $stmt->bindValue(':page', $page, PDO::PARAM_STR);
    $stmt->bindValue(':terms', $query_terms, PDO::PARAM_STR);
    $stmt->execute(); 
    $rows = $stmt->fetchAll();

  2. #2
    Join Date
    Feb 2014
    Location
    south africa
    Posts
    16
    It seems safe,

    a few small tips though :

    trim your inputs (it is a personal thing, but I have found it has saved me many headaches):
    Code:
    if (isset($_GET['filter'])) {$filter = trim($_GET['filter']);} else {$filter = "Customer";}
    if (isset($_GET['terms'])) {$terms = trim($_GET['terms']);} else {$terms = null;}
    have separate white lists (in future they may not be the same)
    Code:
    $filter_whitelist = = array("Customer", "Web", "Tax_ID");
    $sort_whitelist = array("Customer", "Web", "Tax_ID");
    rather than killing the process for invalid input, rather use your defaults
    Code:
    if(!in_array($filter, $filter_whitelist)) {$filter = "Customer";}
    if(!in_array($sort, $sort_whitelist)) {$sort = "Customer";}
    if(!in_array($d, $dir_whitelist)) {die($d = "asc";}
    you are using PDO, so let it do the work on your input variables for you :
    Code:
    $stmt = $db->prepare("SELECT customers.ID, customers.Customer, customers.Size, customers.Status, customers.Web, customers.Contact_Email, resellers.Reseller, distributors.Distributor FROM customers INNER JOIN resellers on customers.Reseller=resellers.ID INNER JOIN distributors on resellers.Distributor=distributors.ID WHERE customers.Status='Prospect' AND filter_1_column_name LIKE ? AND filter_2_column_name LIKE ? ORDER BY ? ? LIMIT :page, 18");
    $stmt->execute(array($filter,$filter_2_value,$sort,$d); 
    $rows = $stmt->fetchAll();

  3. #3
    Join Date
    Aug 2004
    Location
    Ankh-Morpork
    Posts
    19,355
    As far as SQL injection is concerned, the only part of all of that which means anything is where you use bound parameters in your prepared statement. Doing that as you are for any external values will take care of this issue -- everything else is application-specific as to how you want to filter or not filter inputs for other (non-SQL-related) reasons.
    "Please give us a simple answer, so that we don't have to think, because if we think, we might find answers that don't fit the way we want the world to be."
    ~ Terry Pratchett in Nation

    eBookworm.us

Thread Information

Users Browsing this Thread

There are currently 1 users browsing this thread. (0 members and 1 guests)

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •  
HTML5 Development Center



Recent Articles