0

I am using the pagination class below with PDO OOP

<?php
class Paginator{
private $db;
public $page_no;//current page
public $limit;//record_per page
public $row_start;
public $total_rec;
public $query;

function __construct($con){
    $this->db = $con;
}
//get total no of records
public function get_no_records($query){
    $this->query = $query;
    $stmt = $this->db->prepare($query);
    $stmt->execute();
    $row_num = $stmt->rowCount();
    if($row_num > 0){
        $this->total_rec = $row_num;
        return $row_num;
    }
}
public function get_data($limit,$page_no){
    try {
        $this->limit = $limit;
        $this->page_no = $page_no;
        if($this->limit == "all"){
            $query = $this->query;
        }
        else{
            $this->row_start = (($this->page_no-1) * $this->limit);
            $query = $this->query . " LIMIT ". $this->row_start . "," . $this->limit;
        }
        $stmt = $this->db->prepare($query);
        $stmt->execute();
        while($row = $stmt->fetch(PDO::FETCH_ASSOC)){
            //create an array to hold record
            $results[] = $row;
        }
        $result = new stdClass();
        $result->page_no = $this->page_no;
        $result->limit = $this->limit;
        $result->total_rec = $this->total_rec;
        $result->data = $results;
        return $result;
    } catch (PDOException $e) {
        echo $e->getMessage();
    }
}
public function create_links($links,$list_class){
    if($this->limit == 'all'){
        return '';
    }
    $last = ceil($this->total_rec/$this->limit);
    $start = (($this->page_no - $links) > 0) ? $this->page_no - $links : 1;
    $end = (($this->page_no + $links) < $last) ? $this->page_no + $links : $last;
    $html = '<ul class="' . $list_class . '">';
    $class = ($this->page_no == 1) ? "disabled" : "";
    $previous_page = ($this->page_no == 1) ?
    '<a href= ""><li class="' . $class . '">&laquo;</a></li>' :
    '<li class="' . $class . '"><a href="?limit=' . $this->limit . '&page_no=' . ($this->page_no-1) . '">&laquo;</a></li>';
    $html .= $previous_page;
    if($start > 1){
        $html .= '<li><a href="?limit=' . $this->limit . '&page_no=1">1</a></li>';
        $html .= '<li class="disabled"><span>....</span></li>'; 
    }
    for($i = $start;$i<=$end;$i++){
        $class = ($this->page_no == $i)? "active" : "";
        $html .= '<li class="' . $class . '"><a href="?limit=' . $this->limit . '&page_no=' . $i .'">' . $i . '</a></li>';
    }
    if( $end < $last){
        $html .= '<li class="disabled"><span>....</span></li>';
        $html .= '<li><a href="?limit=' . $this->limit . '&page_no=' . $last . '">' . $last . '</a></li>';
    }
    $class = ($this->page_no == $last)? "disabled" : "";

    $next_page = ( $this->page_no == $last)?
    '<li class="' . $class . '"><a href="">&raquo;</a></li>':
    '<li class="' . $class . '"><a href="?limit=' . $this->limit . '&page_no=' . ($this->page_no + 1) . '">&raquo;</a></li>';
    $html .= $next_page;
    $html .= '</ul>';
    return $html;
}
}
?>

From the get_no_records($query) above any query passed is executed,I had a query like SELECT * FROM users and it worked fine. I have a function where the value of the column name is determined by the user input from a text field in a form here is the function

            public function search_user($value){
        $query = "SELECT * FROM users WHERE username = " . "'" . $value . "'";
            return $query;
        }

Here is my search form

<form method="GET">
Username:<input type="text" name="uname"/>
<button type="submit" class="btn btn-primary" name="srch">Search</button>
</form>

The $query returned is passed to get_no_records($query) And it is working Fine.Here is My question. Is it right to send user input to the database that way? Is my code vulnerable to sql injection? How do i prevent this. Thanks.

Muhammed
  • 31
  • 6
  • 1
    As long as you aren't using prepared statements, you are very vulnerable to sql injection – Rotimi Oct 01 '17 at 18:49

1 Answers1

0

You really need to use PDO prepared statements, as it is a reliable way to ensure that your website is safe from SQL Injection.

Reference: https://stackoverflow.com/a/3716402/5287820

wpgbrown
  • 40
  • 9
  • But the pagination script above does not give room for that,Please how do i do this .Since the get_no_records($query) only takes SQL queries – Muhammed Oct 01 '17 at 18:45
  • 1
    Just change it to take a query and an array of params – JimL Oct 01 '17 at 18:49
  • @JimL Thanks. But i am using the pagination class in more than one part of my website, wont passing an array of parameters restrict the use of that class to just one part of my website – Muhammed Oct 01 '17 at 18:52
  • Set it to default to an empty array, the execute method should take an array of values and if it gets none then it will match the number of placeholders (if there are none). Any other query anywhere in the app with variables in the query string should be using params though. But sure, there will probably be a few queries that are just select alls. – JimL Oct 01 '17 at 18:54
  • @JimL Thanks for your reply, I'll try that now – Muhammed Oct 01 '17 at 19:06