1

I am still working on my own database class with pdo:

class Database {

    private $databaseConnection;

    public function __construct($path = "", $dbUsername = "", $dbPassword = ""){
        $parts = explode('.',$path);
        $documentType = array_pop($parts);

        if(($path == "") || ((strcmp($documentType, "sq3") !== 0) && (strcmp($documentType, "sqlite") !== 0))) {
            throw new OwnException("The Database must bee .sq3 or .sqlite and Path must be stated");
        }

        $this->databaseConnection = new PDO('sqlite:' . $path, $dbUsername, $dbPassword);
        $this->databaseConnection->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
        $this->databaseConnection->setAttribute(PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_ASSOC);
        $this->databaseConnection->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);

        self::query('CREATE TABLE IF NOT EXISTS User(
                id INTEGER PRIMARY KEY AUTOINCREMENT,
                username VARCHAR(40) NOT NULL UNIQUE,
                numberoflogins INTEGER DEFAULT 0,
                bannedstatus BOOLEAN DEFAULT FALSE,
                dateofjoining TIME
            )');//password field coming soon

        //self::query('CREATE TABLE...');

        //self::query('CREATE TABLE...');
    }

    private function query($sql, $params = NULL){
        $pdoStatement = $this->databaseConnection->prepare($sql);
        $pdoStatement->execute(array_values((array) $params));
        return $pdoStatement;
    }

    public function getObjects($objectTable, $searchedForAttribute, $attributeValue){
        $pdoStatement = $this->databaseConnection->prepare("SELECT * FROM $objectTable  WHERE  $searchedForAttribute = ?");
        $pdoStatement->setFetchMode(PDO::FETCH_CLASS | PDO::FETCH_PROPS_LATE, $objectTable);
        $pdoStatement->execute(array($attributeValue));
        $resultObjects = array();
        while($resultObject = $pdoStatement->fetch()){
            array_push($resultObjects, $resultObject);
        }
        if(empty($resultObjects)){
            return false;
        }
        return $resultObjects; 
    }

    public function getObject($objectTable, $searchedForAttribute, $attributeValue){
        //...returns only the first Object from getObjects()
    }

    public function insertObject($object){
    $objectTable = get_class($object);
        $objectData = $object->getAttributes();
        return $this->query("INSERT INTO $objectTable("
               . join(',', array_keys($objectData)) . ")VALUES(" 
               . str_repeat('?,', count($objectData)-1). '?)', $objectData);
    }

    public function updateAttribute($objectTable, $setData, $searchedAttribute, $searchedAttributeValue){
    ...
    }

    public function updateObject($object){
    ...
    }

    public function attributeRemoveObject($objectTable, $searchedForAttribute, $attributeValue){
    ...
    }

    public function __destruct(){
        unset($this->databaseConnection);
    }
}

as you can see there is still no data validation for the functions (and no exception handling, work in progress) such like getObjects() so the variables $objectTable, $searchedForAttribute and $attributeValue going direct into the query. This means no protection against SQL injections.

So I thought it would be quite helpful if I use a static function to validate data before inserting into query:

public static function validate($unsafeData){
    //validate $unsafeData
    return $safeData
    }

Because I want to have the ability to search for usernames with similar names and stuff bin2hex() and hex2bin() is a bad choice and for some attributes like the username it is easy to find some starting points for the validation. For instance I would search for empty space, ', " and =...

But how should I validate the content of a forumpost which contains a lot of signs used for SQL queries to? I mean it could also be a a post about sql itself.

I saw a lot of examples for SQL Injections but all of them missing the point that the main manipulation could also be in the content box.

So how does a forum prevent SQL Injections and Errors referring to the content of a post ?

goulashsoup
  • 2,639
  • 2
  • 34
  • 60

1 Answers1

1

Your weakest point is here:

public function insertObject($object){
    $objectTable = get_class($object);
        $objectData = $object->getAttributes();
        return $this->query("INSERT INTO $objectTable("
               . join(',', array_keys($objectData)) . ")VALUES(" 
               . str_repeat('?,', count($objectData)-1). '?)', $objectData);
    }

The best way to avoid SQL injection is using PDO::bindParam. It doesn't matter if a string field contains valid SQL as long as you use prepared queries and bound parameters:

$pdo->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);
$pquery = $pdo->prepare(
    'INSERT INTO table(column1, column2) VALUES(:column1, :column2)');

// PDO::PARAM_INT, PDO::PARAM_STR, PDO::PARAM_BOOL, etc.
$pquery->bindValue(':column1', $column1, PDO::PARAM_INT); // error if $column1 isn't integer value
$pquery->bindValue(':column2', $column2, PDO::PARAM_STR); // string are sanitized
$pquery->execute();

For an arbitrary object you have to use some sort of metadata to select the correct PDO::PARAM_X value (default is PDO::PARAM_STR):

<?php

class User
{
    public $username = 'foo\'; DROP TABLE User; --';
    public $email    = 'bar@gmail.com';
    public $age      = 500;
}

function getColumnType()
{
    return PDO::PARAM_STR; // just for demo
}

$object = new User;

$ref = new ReflectionObject($object);
$table = $ref->getShortName(); // to avoid FQN
$properties = $ref->getProperties(ReflectionProperty::IS_PUBLIC);

$params  = []; $columns = [];
foreach ($properties as $property) {
    $params[]  = ':'.($columns[] = $property->getName());
}

// in memory db for demo
$pdo = new PDO('sqlite::memory:');
$pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
$pdo->exec('create table User(id INTEGER PRIMARY_KEY, username VARCHAR(250) NOT NULL,email VARCHAR(250) NOT NULL,age INT)');

// your answer
$pdo->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);
$pquery = $pdo->prepare("INSERT INTO $table(".implode(',', $columns).") VALUES(".implode(',', $params).")");

foreach ($properties as $property) {
    $paramName = ':'.$property->getName();
    $paramValue = $property->getValue($object);
    $paramType = getColumnType($object, $property); // return PDO::PARAM_X
    $pquery->bindValue($paramName, $paramValue, $paramType);
}

$pquery->execute();

// check
$all = $pdo->prepare('select * from User');
$all->execute();
var_dump($all->fetchAll(PDO::FETCH_CLASS, 'User'));

Output:

array(1) {
  [0] =>
  class User#10 (4) {
    public $id =>
    NULL
    public $username =>
    string(25) "foo'; DROP TABLE User; --"
    public $email =>
    string(13) "bar@gmail.com"
    public $age =>
    string(3) "500"
  }
}

You must implement getColumnType to get the correct column data type, for example, parsing annotated comments. But at this point you better use some ORM like Eloquent or Doctrine.

Absalón Valdés
  • 910
  • 7
  • 16