PHP insert SQL

[Hikaru12]

This may seem very basic but not sure why I'm having issues with this. I'm trying to create a basic bookmark manager (as I don't like the idea of Google storing aka. "syncing" my bookmarks for an indefinite period of time)

 

Here's what I got

<h1>Bookmarks Manager</h1><form action="insertform.php" method="post">Bookmark Name: <input type="text" name="title"><br /><br/>Bookmark URL: <input type="text" name="url"><br /><br/>Category: <input type="text" name="category"><br /><br />Notes: <textarea name="notes" rows="3" cols="25"></textarea><br /><br/><input type="submit" name="submit"></form><?phpif (isset($_POST['submit'])) {$con = mysql_connect("localhost", "root", "flame123");if (!$con){die("Can not connect: " . mysql_error());	}	mysql_select_db("bookmarks", $con);$sql = "INSERT INTO bookmarkstable(Title,URL,Category,Notes) VALUES ('$_POST[title]','$_POST[url]','$_POST[category]','$_POST[notes]'";mysql_query($sql,$con);mysql_close($con);}

The MYSQL table looks like the following:

id - int(3) unsigned primary key autoincrement

BookmarkTitle - varchar(50)

URL - Text

Category - varchar(50)

Notes - Text

 

Do the parameters for INSERT INTO have to match the database column names? e.g. Title should actually be BookmarkTitle from above code:

$sql = "INSERT INTO bookmarkstable(Title,URL,Category,Notes) 

[vorticalbox]

$Sql = ("Insert into tablename (BookmarkTitle, URL, Category, notes) value (data from form post)";)

[Hikaru12]

$Sql = ("Insert into tablename (BookmarkTitle, URL, Category, notes) value (data from form post)";)

 

Thanks, simple fix.

[vorticalbox]

Thanks, simple fix.

It's values not value. I'm on my phone so coding isn't exactly great lol

[Neil]

Please don't write code like this. Please.

 

http://webdevrefinery.com/forums/topic/1272-your-mysql-code-sucks/

 

Read that.

 

And never, ever, ever write mysql_* again. Because I can take down all of your bookmarks with the following in any of your fields:

');DROP DATABASE bookmarks;--

Or any other number of blind SQL injections.

[keja]

Please don't write code like this. Please.

 

http://webdevrefinery.com/forums/topic/1272-your-mysql-code-sucks/

 

Read that.

 

And never, ever, ever write mysql_* again. Because I can take down all of your bookmarks with the following in any of your fields:

');DROP DATABASE bookmarks;--

Or any other number of blind SQL injections.

 

No you cant, that injection is not going to work.

 

 

but you are right, no point in using deprecated libraries

[asquirrel]

Yea...that code needs a lot of work. First, use this function to get your PHP variables via POST in a SQL safe manner:

#Returns the $_POST value as an array if the variable is set. False otherwise.#[0] = PHP/HTML escaped#[1] = mysql escapedfunction get_POSTValue($mysql,$name){    $tmp=array("","");    if( isset($_POST["$name"]) )    {        $tmp[0] = htmlspecialchars($_POST["$name"]);        $tmp[1] = mysqli_real_escape_string($mysql,$_POST["$name"]);        if( $tmp[0] === "" && $tmp[1] === "" ) { return false; }        return $tmp;    }    return false;}

This gets around the problem of needing to escape some characters in HTML but not in SQL and vice-versa. It just does both escape versions and expects you to be smart enough to use the correct array. As for how to use it in a page:

$u_name=get_POSTValue($mysql,"name");

Where $mysql is the result of a mysqli_connect() call and "name" is the HTML field name in the form. You would then use the variable like this:

$sql = "UPDATE asset_names SET name = '" . $u_name[1] . "' WHERE asset_id = " . $u_asset[1] . ";";

for a SQL query or 

print("<p>Error updating asset name: " . $u_name[0] . ".</p>");

for an HTML escaped version.

[SSL]

#Returns the $_POST value as an array if the variable is set. False otherwise.#[0] = PHP/HTML escaped#[1] = mysql escapedfunction get_POSTValue($mysql,$name){    $tmp=array("","");    if( isset($_POST["$name"]) )    {        $tmp[0] = htmlspecialchars($_POST["$name"]);        $tmp[1] = mysqli_real_escape_string($mysql,$_POST["$name"]);        if( $tmp[0] === "" && $tmp[1] === "" ) { return false; }        return $tmp;    }    return false;}

 

What the brown? You should be using PDO and prepared statements. In particular doing things this way will not adequately protect from second order SQL injection.

[asquirrel]

What the brown? You should be using PDO and prepared statements. In particular doing things this way will not adequately protect from second order SQL injection.

1. PDO is a matter of taste. I'm a C programmer who learned to do PHP for one specific thing (the project I quoted my own code snippet from). PDO is a waste of my brain cycles, I prefer functional programming.
2. Prepared statements would work for that specific query, but I never got them to work.
3. If you can actually manage to get a second order PHP injection to work against the code provided, please provide an example. While I am an idiot at PHP I did take the time to not leave (obvious) security flaws in my design. I had a few guys from RSA validate my pages were secure, they were impressed someone as ignorant about PHP as I was made something their tools said was ok. *shrug*

[SSL]

1. PDO is a matter of taste. I'm a C programmer who learned to do PHP for one specific thing (the project I quoted my own code snippet from). PDO is a waste of my brain cycles, I prefer functional programming.
2. Prepared statements would work for that specific query, but I never got them to work.
3. If you can actually manage to get a second order PHP injection to work against the code provided, please provide an example. While I am an idiot at PHP I did take the time to not leave (obvious) security flaws in my design. I had a few guys from RSA validate my pages were secure, they were impressed someone as ignorant about PHP as I was made something their tools said was ok. *shrug*

 

Obviously a second order wouldn't work against the code you provided. The situation where second order would be of concern does not arise. But this kind of programming with PHP can easily lead to that kind of situation - and I'm thinking of other people who will read your post, not just you, who may think that this is a good way to construct software with PHP.

 

Your ability to get a prepared statement to work is irrelevant. Fact remains that prepared statements are the best practice method for constructing secure SQL queries with PHP. Using "mysqli_real_escape_string" manually everywhere is cumbersome and prone to error, and no, "htmlspecialchars" is not an XSS filter. I absolutely do not care that a simplistic PHP program you wrote managed to pass some test suite. Again, I'm concerned with demonstrating good practices given that this is a forum of mostly beginners. Your lazy, hacked-together abomination of a PHP program is not an example of that.

[asquirrel]

Obviously a second order wouldn't work against the code you provided. The situation where second order would be of concern does not arise. But this kind of programming with PHP can easily lead to that kind of situation - and I'm thinking of other people who will read your post, not just you, who may think that this is a good way to construct software with PHP.

 

Your ability to get a prepared statement to work is irrelevant. Fact remains that prepared statements are the best practice method for constructing secure SQL queries with PHP. Using "mysqli_real_escape_string" manually everywhere is cumbersome and prone to error, and no, "htmlspecialchars" is not an XSS filter. I absolutely do not care that a simplistic PHP program you wrote managed to pass some test suite. Again, I'm concerned with demonstrating good practices given that this is a forum of mostly beginners. Your lazy, hacked-together abomination of a PHP program is not an example of that.

That is an unreasonable amount of anger and personal insult. Calm down, abstract. If you want to teach others, and have them learn, then provide code examples as I have. Currently all you've done is provided is an attempt at swearing, a good measure of anger, and insults. This makes me not want to learn from you, in fact, it makes me want to add you to my ignore list. Your first contributions to this thread did not contribute to the main discussion and have required me to derail the discussion to defend myself. You have not contributed anything to the OP other than white noise.

 

I protect against XSS by not ever loading CSS from a variable, any CSS I load is statically defined. My website is *real* low tech. As for XSS, according to: http://stackoverflow.com/questions/19584189/when-used-correctly-is-htmlspecialchars-sufficient-for-protection-against-all-xI am perfectly fine as long as I don't use an javascript functions (onmouse, javascript calls, etc) which I don't, and as long as I quote all fields inserted into the page, which I do. So...I'm 2/2 on security until you can provide an example.

 

As for second order attacks, I protect against those with good database schema design. Any value taken from a user can be written to the database, but that value, when read, is only ever provided to HTML. It is never valid for a UNION/JOIN command, WHERE clause, etc. This requires anyone who modifies my code to have an IQ greater than that of a college freshman, but that's an ok trade-off to me and my management.

 

On mysqli_real_escape_string() {henceforth called mres()}: PDO + Prepared strings only protects you if you use them perfectly. You can make several mistakes that are all valid PHP syntax which look like prepared statements and yet offer you no or limited security. An example of such a thing would be using $_GET[] in your bind call.

 

As for whether mres() is secure or not, an interesting discussion on that topic can be found here http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string/12118602#12118602The gist of that post is that, yes, mres() can be bypassed, but only if you fail to ensure that the client and the server are using the same character set. That said, my database and connection both use utf8, which is not vulnerable to the by-pass POC presented.

 

On security: If someone copy-pasted my code they might be vulnerable, but if they are building a production website via LTT forum posts...well, just remember what happened when people on ~a bitcoin forum~ (I don't remember which one) helped get Mt.Gox setup. I saw screenshots of cleartext passwords sent via HTTP GET on a production version of Mt.Gox's website. I mean...any idiot can steal code. There's no way to copy-paste secure code. You should know that. Security is a mindset, and you either have it or you don't. I could post here all day and never give a piece of code that was absolutely secure. Computers don't work that way. Security doesn't work that way. Not right this second anyhow; I wait to see what the future holds.

 

In closing: I love teaching and sharing knowledge. I will be wrong from time to time as any human is. I will not post any further defense of my work as the current defense is sufficient and any additional discussion would derail from OP's original request.

 

edit: I wrote my reply, hit the submit button, and about 10 minutes later realized XSS != CSS. That was my derp. Good thing I said I'm only a human and make mistakes! I know XSS is any kind of scripting. I had a blonde moment. Post updated to reflect reality.

Edited June 3, 2015 by asquirrel

[SSL]

That is an unreasonable amount of anger and personal insult. Calm down, abstract. If you want to teach others, and have them learn, then provide code examples as I have. Currently all you've done is provided is an attempt at swearing, a good measure of anger, and insults. This makes me not want to learn from you, in fact, it makes me want to add you to my ignore list. Your first contributions to this thread did not contribute to the main discussion and have required me to derail the discussion to defend myself. You have not contributed anything to the OP other than white noise.

If that's the only way you can characterize it, you apparently haven't understood my point, or lack the ability to reflect on your own.

 

I protect against XSS by not ever loading CSS from a variable, any CSS I load is statically defined. My website is *real* low tech.

 

XSS has nothing to do with CSS. And as I stated previously, your specific implementation doesn't matter and isn't what interests me.

 

As for second order attacks, I protect against those with good database schema design. Any value taken from a user can be written to the database, but that value, when read, is only ever provided to HTML. It is never valid for a UNION/JOIN command, WHERE clause, etc. This requires anyone who modifies my code to have an IQ greater than that of a college freshman, but that's an ok trade-off to me and my management.

Good schema design has nothing to do with using values from the database to query the database. If it did, databases would not be very useful. A simple application may be able to get by with this handicap, but it's a matter of course in real software.

 

On mysqli_real_escape_string() {henceforth called mres()}: PDO + Prepared strings only protects you if you use them perfectly. You can make several mistakes that are all valid PHP syntax which look like prepared statements and yet offer you no or limited security. An example of such a thing would be using $_GET[] in your bind call.

The whole point of bound parameters is that no additional sanitization or escaping is necessary from the perspective of database security. While the application may need to impose constraints on valid input, that issue has no bearing on SQL injection vulnerability.

 

As for whether mres() is secure or not, an interesting discussion on that topic can be found here http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string/12118602#12118602The gist of that post is that, yes, mres() can be bypassed, but only if you fail to ensure that the client and the server are using the same character set. That said, my database and connection both use utf8, which is not vulnerable to the by-pass POC presented.

My point about using mres has nothing to do with the inherent security of the function but with the ease of use, which also affects security. Your example provides no contradiction since by your own admission your site is simple and "low tech".

 

On security: If someone copy-pasted my code they might be vulnerable, but if they are building a production website via LTT forum posts...well, just remember what happened when people on ~a bitcoin forum~ (I don't remember which one) helped get Mt.Gox setup. I saw screenshots of cleartext passwords sent via HTTP GET on a production version of Mt.Gox's website. I mean...any idiot can steal code. There's no way to copy-paste secure code. You should know that. Security is a mindset, and you either have it or you don't. I could post here all day and never give a piece of code that was absolutely secure. Computers don't work that way. Security doesn't work that way. Not right this second anyhow; I wait to see what the future holds.

Not worried about people copying code verbatim, though that is a concern as well. Mainly, it's using code they see as a model, and not necessarily immediately. Since you're clearly an experienced programmer, I'm sure you know how easily bad habits can persist if the right knowledge is not ingrained early.

[asquirrel]

@SSL you are indeed correct that XSS != CSS. I realized that a few minutes after I posted it, but it wasn't until just now I had the time to fix my post. That was a blonde moment for me. My post has been edited to reflect reality. As for your attempted character assination of me...hurray ignore functionality.

[SSL]

hurray ignore functionality.

 

Too bad, I enjoyed our little tete-a-tete.