-
Active Member
Last edited by Valkryst; 10-04-2023 at 12:15 PM.
-
Post Thanks / Like - 1 Thanks
Byakurai (1 members gave Thanks to Valkryst for this useful post)
-
Did you check this for sqli?
-
Active Member
Originally Posted by
homer91
Did you check this for sqli?
SQL Injection? Yeah, it should be safe from it. No special characters are allowed and everything important is done server-side so I can't think of any way for someone to mess with it.
-
Thanks for this guide. Didn't see it before and a up to date account registration script has been needed for a while.
I am really not confident about this piece of code though:
Code:
include('config.inc.php');
$username = $_POST['accUsername'];
$password = $_POST['accPassword'];
$connection = new mysqli($host, $user, $pass, $db, $port);
mysqli_query($connection, "call create_account(\"{$username}\", \"{$password}\");");
$connection->close();
Any time you take input directly from POST/GET or any other client data you should really verify it.
It should be something like:
Code:
$string = mysql_real_escape_string($string);
My PHP knowledge is not that great though so I could be wrong.
-
Sergeant
Very useful & Interesting, thanks for this.
-
Active Member
@stoneharry
I did a quick search on the mysql_real_escape_string function and was told that it "Escapes special characters in a string for use in an SQL statement.". I'm going to assume that this serves the same purpose as the code below (a snipit of code from the OP). What I did, because I barely know php, is to prevent any of the special characters from being used in usernames/passwords as you can see <removed the link>.
I'll take a better look at that function later today and test it out. If it works as I think it does then I'll do a quick rewrite of the OP.
Thanks for the tip!
Last edited by Valkryst; 05-15-2014 at 03:22 PM.
-
Originally Posted by
Valkryst
@stoneharry
I did a quick search on the
mysql_real_escape_string function and was told that it "Escapes special characters in a string for use in an SQL statement.". I'm going to assume that this serves the same purpose as the code below (a snipit of code from the OP). What I did, because I barely know php, is to prevent any of the special characters from being used in usernames/passwords as you can see
here.
I'll take a better look at that function later today and test it out. If it works as I think it does then I'll do a quick rewrite of the OP.
Thanks for the tip!
Since you are specifically using direct POST data input, you can use this to modify the POST data sent to the website: https://addons.mozilla.org/en-US/fir...n/tamper-data/
(I don't have FireFox installed and Chrome is difficult to do this in.)
Modifying the post data directly will bypass any type of sanitisation you have done previously.
-
Contributor
Instead of using mysql_real_escape_string, you should be using prepared statements.
-
Active Member
Originally Posted by
InternetExplorer
Instead of using mysql_real_escape_string, you should be using prepared statements.
I took a quick look at PHP: Prepared statements and stored procedures - Manual and I don't really see why they would be any better than mysql_real_escape_string. Care to explain?
-
Contributor
Originally Posted by
Valkryst
mysql_real_escape string is deprecated, and will be removed from PHP in the future.
With prepared statements, SQL statements are sent & parsed to your database separatly from any parameters, which makes it impossible to inject any malicious SQL.
mysql_real_escape_string only escapes special characters and still leaves your query at risk for SQL injection, as user provided data will compose your SQL statement.
-
Active Member
Drat. Prepared statements do sound like a necessary modification to the code, but I've forgotten nearly everything about PHP that I knew when writing most of this. It was more of a "code for 24-hours straight, get it to work, then forget everything" kind of deal. =/
Would you care to modify the code to work with a prepared statement?
-
Contributor
Originally Posted by
Valkryst
Drat. Prepared statements do sound like a necessary modification to the code, but I've forgotten nearly everything about PHP that I knew when writing most of this. It was more of a "code for 24-hours straight, get it to work, then forget everything" kind of deal. =/
Would you care to modify the code to work with a prepared statement?
I've never used Mysqli, so I only know how to do this with PDO.
confic.inc.php:
Code:
<?php
class MySQL {
public static $dsn = 'mysql:host=HOST NAME;dbname=DB NAME;charset=utf8';
public static $user = '';
public static $pass = '';
private static $db;
final private function __construct() { }
final private function __clone() { }
public static function get() {
if (is_null(self::$db)) {
self::$db = new PDO(self::$dsn, self::$user, self::$pass);
self::$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
self::$db->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);
}
return self::$db;
}
}
?>
Call your connection with:
Code:
require('config.inc.php');
$db = MySQL::get();
Then you can do SELECT queries like this:
Code:
$stmt = $db->prepare('SELECT username FROM account WHERE username = ?');
$stmt->execute(array($_POST['accUsername']));
$rows = $stmt->fetchAll(PDO::FETCH_ASSOC);
if ($rows) {
// user found, do something
} else {
// not found
}
INSERT INTO queries:
Code:
$stmt = $db->prepare('INSERT INTO account(username, password) VALUES (?,?)');
$stmt->execute(array($_POST['accUsername'], $_POST['accPassword']));
PHP will automaticly close the connection itself when script is finished.
Btw, your form is vulnerable to CSRF attacks.
-
Active Member
Thanks a lot for writing out that code. I'm working on getting a test-version of the code up and running at the moment before I rewrite the tutorial with it.
I've just-about got the code ready for testing. For the INSERT query I'm actually using a stored procedure instead, but that's not too relevant to this next bit.
Basing my slightly altered test-code off of this:
Code:
$stmt = $db->prepare('INSERT INTO account(username, password) VALUES (?,?)');
$stmt->execute(array($_POST['accUsername'], $_POST['accPassword']));
Should it be written like this if I'm using variables:
Code:
$accUsername = $_POST['accUsername'];
$accPassword = $_POST['accPassword'];
$stmt = $db->prepare('CALL create_account(:accUsername, :accPassword);');
$stmt->bindParam(':accUsername', $accUsername);
$stmt->bindParam(':accPassword', $accPassword);
Edit:
I can't seem to get it working correctly. Maybe I've combined the different pieces of code you've given me incorrectly?
As far as my tests have shown, the echo on line 96 doesn't show up after submitting the form and the accounts are never created even after submitting correct information. I'll keep messing around for now.
Current HTML/PHP code: http://pastebin.com/xa9CdcTy
confic.inc.php: http://pastebin.com/rKFuqK9Q
Thanks again for your help.
Last edited by Valkryst; 05-16-2014 at 10:44 PM.
-
Contributor
You forgot to execute the query.
$stmt->execute();
-
Active Member
Originally Posted by
InternetExplorer
You forgot to execute the query.
$stmt->execute();
Both execute statements are there from what I see. Lines 88 and 119 contain the execute statements. Was there another place where there had to be one?