HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Injections and query

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
andqueryinjections

Problem

I made a class that connects to my DB and inserts some values. Is it secure or how can I protect this further from injections? The object declaration will come from variables with POST from a form, after being validated, ofc. Just want to know if this is secure.

That, and also, should I make a method for every query I need? Or is there a better and secure way?

server=$server;
        $this->user=$user;
        $this->pass=$pass;
        $this->name=$name;
    }

    public function tryconn() {
        $this->conn = new mysqli(  $this->server, $this->user, $this->pass, $this->name );

    if ( mysqli_connect_error()) {
        die( '*************Connection Error (' . mysqli_connect_errno()  . '):'
                .mysqli_connect_error() );
    }

    else echo 'ok';

}

public function query_register( $user, $pass, $email ) {
    $stmt = $this->conn->prepare( "INSERT INTO `users` (`username`, `password`, `email`) VALUES (?, ?,?)" );
    $stmt->bind_param( "sss", $user, $pass, $email );
    $stmt->execute();
    $stmt->close();
}

}//end of class

$a=new WorkDB( $DBServer, $DBUser, $DBPass, $DBName );
$a->tryconn();
$a->query_register( 'a', 'b', 'c' );

?>

Solution

What you did well

  • Storing sensitive information, such as the database connection parameters, separately from the source code is a good idea.



  • You used parameterized queries with placeholders, which are not vulnerable to SQL injection.



Things to work on

  • Your indentation is inconsistent.



  • tryconn() should not echo 'ok' on success, as that pollutes the output.



  • $a is a horrible name for your database handle. It's conventionally called $db or something like that.

Context

StackExchange Code Review Q#54311, answer score: 6

Revisions (0)

No revisions yet.