security related

[EricOnAdventure]

Hey guys, I wanted to know if this was safe. After a user logs in, and I want to get some information I have an SQL function and part of it goes

 

$username = $_SESSION[info]['username'];        

$query = "SELECT $column FROM $table WHERE username ='$username' limit 1";

 

I am also using INSERT INTO sometimes

 

Because I am getting the username from the session is there a security issue I should be aware of?

 

[Jacques1]

Why should a value from a session automatically be SQL-safe? A session can contain anything. In fact, I would expect a username to contain single quotes, or else you'll have to ban the entire population of Ireland (O'Reilly, O’Neill, ...).

 

Stop playing with SQL injection vulnerabilities and just use prepared statements for all dynamic input. At all times. No exception.

[Muddy_Funster]

The answer to questions like this depends a lot on the situation.  Is it the most secure way of doing things? No.  But, you need to consider how much security is practical and relevant for the data that you are storing.  If all you have is a bunch of arbitrary information on people that could easy be gotten just from looking up their facebook link then spending weeks securing your site isn't a productive use of time.  I personally don't hold relevant human readable data in the session.  I base64 encode relative info - like a pre-hashed user UID token- so that nothing is immediately obvious to people looking at the developer tools that are now included with every browser.  I then use this data to pull up related data from the database ad-hock.

 

Using username as your conditional means that anyone that knows another persons username (often visible during login and displayed in account info and the such after it) and knows how to hack the session cookie (not a big challenge these days) could spoof that user directly.  That could be a big deal, or it could only let someone access someones real first name and country of residence.

 

It's all relative.  For example using variables to define column and table names directly within the query string makes me shudder, but you, and others, are happy enough to do it.  I would rather define absolute queries using hundreds of conditionals than throw more variables into a raw query string that I absolutely needed to.

[Jacques1]

You must be kidding.

 

So you're fine with leaking the personal data of your users “because it's already on Facebook”? Is that your attitude as a professional developer? Is that how you write applications?

 

Also, you clearly don't know what an SQL injection vulnerability means. A single query can be used to take over the entire server, especially when the person responsible for the application thinks that “security isn't really such a big deal”.

[EricOnAdventure]

So since hacking a session is an issue, I of course need to protect my database from what is coming in with a real_escape_string();

Next your suggesting I should hash the UserID, you have a good point but as a user would have to already be logged in to even access this or similar code, and the login is rather secure, would I need to do that, or would it, as it is, be secure enough?

[NotionCommotion]

So since hacking a session is an issue, I of course need to protect my database from what is coming in with a real_escape_string();

 

Next your suggesting I should hash the UserID, you have a good point but as a user would have to already be logged in to even access this or similar code, and the login is rather secure, would I need to do that, or would it, as it is, be secure enough?

 

It is good that you are aware of real_escape_string(), however, no one uses it anymore.  Instead, use a prepared statement ideally PDO.  While there is fancy binding and the like, start off simple.

$username = $_SESSION[info]['username'];        
$query = "SELECT $column FROM $table WHERE username =? limit 1"; //Why not hard code your column and table names?
$stmt->$yourDataBaseConnection->prepare($query);
$stmt->execute([$username]);
$rs=$stmt->fetch(); //Or fetchAll() or fetchColumn().  Also, add parameters such as PDO::FETCH_OBJ.

[Muddy_Funster]

@Jaques1

 

No, now you're twisting things just to be offensive.  What I'm saying is that spending weeks coding to protect information that is already publicly available from being made publicly available is a waste of resources.

 

You really do get quite unpleasant on this subject.  

 

I know enough about SQL injection attacks that I didn't actually stipulate "SQL" in my post, and I also know that an attack does not have to be able to exploit the "ENTIRE SERVER" to be classed as an SQL injection attack - in fact, I would be really fracking impressed if someone gained root access to an "entire server" from a single injection attack.

 

I get that you have a point to get across, and that you clearly are impassioned by the subject of cyber security, but instead of being so abrasive why don't you try and put your point across in a more civil manner?  It will make dialogue more pleasant and in turn make people far more receptive to your point of view.

 

@EricOnAdventure - what @Jacques rightly said is that you should be using mysqli prepared statements It's not quite the same thing.

I'm suggesting, apparently incorrectly, that you should code security appropriate to your data.  However I am disinclined to offer any further suggestions on this subject as I'm really not having the kind of week where I can be bothered faceless person berate me over the internet.

[EricOnAdventure]

 

It is good that you are aware of real_escape_string(), however, no one uses it anymore.  Instead, use a prepared statement ideally PDO.  While there is fancy binding and the like, start off simple.

$username = $_SESSION[info]['username'];        
$query = "SELECT $column FROM $table WHERE username =? limit 1"; //Why not hard code your column and table names?
$stmt->$yourDataBaseConnection->prepare($query);
$stmt->execute([$username]);
$rs=$stmt->fetch(); //Or fetchAll() or fetchColumn().  Also, add parameters such as PDO::FETCH_OBJ.

The columns and tables are not hard coded because this is part of a function that should help me pull only 1 $var at a time.

Thanks for the advice, I'll work on applying it

[EricOnAdventure]

 

It is good that you are aware of real_escape_string(), however, no one uses it anymore.  Instead, use a prepared statement ideally PDO.  While there is fancy binding and the like, start off simple.

$username = $_SESSION[info]['username'];        
$query = "SELECT $column FROM $table WHERE username =? limit 1"; //Why not hard code your column and table names?
$stmt->$yourDataBaseConnection->prepare($query);
$stmt->execute([$username]);
$rs=$stmt->fetch(); //Or fetchAll() or fetchColumn().  Also, add parameters such as PDO::FETCH_OBJ.

 Could you provide this in mysqli...my code is mysqli and I have a sight grasp over that...PDO is still beyond me.

[Jacques1]

Have you looked at the example code in the PHP manual? By the way, PDO is a lot easier than mysqli, especially when dealing with prepared statements. So unless you've reached the point-of-no-return, you should seriously consider switching to PDO.

[Muddy_Funster]

 Could you provide this in mysqli...my code is mysqli and I have a sight grasp over that...PDO is still beyond me.

 

I don't use mysqli myself, but as best as I can work it out:

 

 

...
$mysqli = new mysqli ('host', 'user', 'pwd', 'schema');
$qry = "SELECT $column FROM $table WHERE username = ? limit 1";
$stmt = $mysqli->prepare($qry);
$stmt->bind_param('s',$username);
$stmt->execute();
...

[benanamen]

@muddy_funster, I have gained complete access to a server through an SQL injection exploit. It is really not difficult if you know what you're doing. Security should always be taken seriously.

Edited June 23, 2016 by benanamen

[ginerjm]

If you are writing a function to pull one column at a time and plan on using it to repeatedly pull different columns from your database, that is truly a poor use of resources. Do not plan on pulling data one column at a time.

[Muddy_Funster]

@muddy_funster, I have gained complete access to a server through an SQL injection exploit. It is really not difficult if you know what you're doing. Security should always be taken seriously.

 

I do take it seriously, and if you were able to break through the mysql user level and get root control over a sever without previously knowing the root user password from passing in an sql command to the mysql application I do genuinely take my hat off to you, hell I'm even impressed to know you can keep interactivity with the server after passing from the sql daemon into the core OS.  This merits some duckduckgo-ing.

[benanamen]

 

 

I'm even impressed to know you can keep interactivity with the server after passing from the sql daemon into the core OS

 

Don't be. That's not even how I did it.

 

* @Jaques1 is just being @Jaques1 and he is right to be serious about security related issues. Even a computer that is not plugged in the wall and not connected to the net needs to be secured.

[NotionCommotion]

@Jaques1

 

No, now you're twisting things just to be offensive.  ...

 

You really do get quite unpleasant on this subject.  

 

What are you talking about?  This is the new and improved Jaques1!  Now, maybe a few years ago...

 

Joking aside, go with his suggestion to learn PDO and prepared statements.

[benanamen]

I don't care how @Jaques1 responds (It took a few posts  ). His knowledge of Security is NSA, CIA, & MI6 level expertise. Where ever he learned it, it wasn't from the Php Manual.

Edited June 23, 2016 by benanamen

[EricOnAdventure]

If you are writing a function to pull one column at a time and plan on using it to repeatedly pull different columns from your database, that is truly a poor use of resources. Do not plan on pulling data one column at a time.

 

The function only exits to get 1 $var at a time, I may use it at most 2x at once. But usually just once.

 

Have you looked at the example code in the PHP manual? By the way, PDO is a lot easier than mysqli, especially when dealing with prepared statements. So unless you've reached the point-of-no-return, you should seriously consider switching to PDO.

Ok thanks, I will move to PDO.