1
\$\begingroup\$

I'm just writing my first PHP-Mysqli sample (think about a Wiki 0.0.1) and I would like to ask you if this example is secure or not or if there are any other problems/suggestions you might recommend?

I would like to use prepared statements and not care about sanitizing the input from $_GET, but I don't know if this code is considered secure and OK?

Also, any comments regarding how this functionality (reading a latest revision from a database) should be done is welcome.

require_once 'connect.php';

$id = $_GET['id'];

if ( $stmt = $mysqli->prepare("SELECT text, rev FROM wiki WHERE id = ? ORDER BY rev DESC LIMIT 0,1") ) {

    $stmt->bind_param( "s", $id );
    $stmt->execute();
    $stmt->bind_result( $text, $rev );
    $stmt->fetch();

    echo "rev: " . $rev . ": " . $text;
    echo '<br>';

    $stmt->close();
}

$mysqli->close(); 

connect.php

$mysqli = new mysqli("localhost", "xxx", "yyy", "zzz");

if (mysqli_connect_errno()) {
    printf("Connect failed: %s\n", mysqli_connect_error());
    exit();
}

Is it safe to keep connect.php at the same folder as the other php files?

Is there any chance someone could read the plain password in connect.php file?

\$\endgroup\$
  • \$\begingroup\$ Don't print errors out so they are visible. 1) It has no meaning to a user (my mum has no idea what a connect failure is nor how to resolve the problem) and makes your site look bad. 2) It provides information to attackers that they do not need. Errors messages should be placed in the log file. The only error message you should display to a user an apology (with an optional guid (that you generate)) and a potential solution (like pleas try latter). If they complain you can use the guid to look up the real error in the log and then resolve the issue. \$\endgroup\$ – Martin York Apr 17 '12 at 0:33
1
\$\begingroup\$

Yes, that looks good.

Minor comment: Normally an id is an integer. Should it have been "i" in your bind_param?

Connection Details

First, I am not a security expert. However, I would recommend storing the connection details in a location that is not served by your web-server. It should be somewhere in your PHP include_path or accessible from your autoloader.

Having the file in a directory accessible from your web-server could open up the connection details if you had an incorrectly configured web-server.

\$\endgroup\$
1
\$\begingroup\$

Verify the id to be an integer. One way to do that is like

$id = (int)$_GET['id'];
if($id > 0) {
 .... //proceed
}

Next, the parameter should be bind using i not s

$stmt->bind_param( "1", $id );
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.