1. Advertising
    y u no do it?

    Advertising (learn more)

    Advertise virtually anything here, with CPM banner ads, CPM email ads and CPC contextual links. You can target relevant areas of the site and show ads based on geographical location of the user if you wish.

    Starts at just $1 per CPM or $0.10 per CPC.

Do I need "mysql_real_escape_string" for this?

Discussion in 'PHP' started by webgames247, Mar 11, 2015.

  1. #1
    Hi,

    like the title said, do i need add "mysql_real_escape_string" for the following.. if yes? how to do it?

    $category_sql = mysql_query("SELECT name, catid FROM categories");

    $sql = mysql_query("SELECT * FROM categories WHERE catid = '$catid'");

    and

    $result = mysql_query($query);

    Thanks
     
    webgames247, Mar 11, 2015 IP
  2. sarahk

    sarahk iTamer Staff

    Messages:
    28,500
    Likes Received:
    4,460
    Best Answers:
    123
    Trophy Points:
    665
    #2
    for starters you shouldn't be using the mysql_ commands as they are deprecated and insecure

    Accepting that you are using them...

    $category_sql = mysql_query("SELECT `name`, `catid` FROM `categories`");
    No need to escape

    $sql = mysql_query("SELECT * FROM `categories` WHERE `catid` = '{$catid}'");
    You need to escape $catid to ensure there's no sql injection in there

    $result = mysql_query($query);
    $query could be anything - unless you can be 100% sure there's not user generated data like $catid in the query you need to escape it.
     
    sarahk, Mar 11, 2015 IP
  3. Ben Cousins

    Ben Cousins Active Member

    Messages:
    11
    Likes Received:
    1
    Best Answers:
    0
    Trophy Points:
    73
    #3
    Only in PHP 5.5+... But yes, they're deprecated and insecure.
     
    Ben Cousins, Mar 11, 2015 IP
  4. sarahk

    sarahk iTamer Staff

    Messages:
    28,500
    Likes Received:
    4,460
    Best Answers:
    123
    Trophy Points:
    665
    #4
    What decent webhost isn't though?
     
    sarahk, Mar 12, 2015 IP
  5. Ben Cousins

    Ben Cousins Active Member

    Messages:
    11
    Likes Received:
    1
    Best Answers:
    0
    Trophy Points:
    73
    #5
    Well the Ubuntu repositories are still on PHP 5.4 and CentOS on 5.3...
     
    Ben Cousins, Mar 12, 2015 IP
  6. PoPSiCLe

    PoPSiCLe Illustrious Member

    Messages:
    4,623
    Likes Received:
    725
    Best Answers:
    152
    Trophy Points:
    470
    #6
    Uhm... mysql_ have been deprecated, and use of it has been discouraged or about... 8 years? 9 years? It's still available, by all means, but it's definitely not recommended.
     
    PoPSiCLe, Mar 12, 2015 IP
  7. deathshadow

    deathshadow Acclaimed Member

    Messages:
    9,732
    Likes Received:
    1,998
    Best Answers:
    253
    Trophy Points:
    515
    #7
    PoPSiCle has it right in that we've been told for over 9 years to STOP using mysql_ functions... pretty much from the day mysqli and PDO were added to the language since THAT'S WHY THEY EXIST!. After all, PDO was introduced in PHP 5.1 in 2005!

    mysql_connect leaves the connection in global scope, and mysql_query has you blindly plugging values into your query strings which has ALWAYS been rubbish. I you are even putting variables into your query (in all but the rarest of cases) you are doing something disastrously insecure and wrong. Same goes for leaving things like the database connection or the connection info in global scope. (Which is why turdpress putting them into DEFINE is some serious herpafreakingderp -- to go with the rest of their "insecure by design" code philosophies!)

    Hence why if you are going to have values in your queries you should be using prepared queries so data and query string are sent separately.

    // assumes $db is a connected PDO object
    $statement = $db->prepare('
    	SELECT * FROM categories
    	WHERE catid = :catId
    ');
    
    $statement->exec([
    	':catId' => $catId
    ]);
    Code (markup):
    Send the data separate from the query, and the types of "code injections" that mysql_real_escape_string are there to pathetically try (and often fail) to prevent are effectively impossible. Fun part of that being you can execute multiple times with different data off the one query.

    ANYONE still teaching mysql_ functions or any source saying to use them is so far out of date, you might as well be vomiting up HTML 3.2 while you are at it. Admittedly, that's what most people still do as well and just slap 4 tranny or 5 lip-service around their outdated code).
     
    deathshadow, Mar 12, 2015 IP
    ThePHPMaster likes this.
  8. Ben Cousins

    Ben Cousins Active Member

    Messages:
    11
    Likes Received:
    1
    Best Answers:
    0
    Trophy Points:
    73
    #8
    From an absolute beginner standpoint it is good to learn basic SQL commanding by using mysql_connect (like I did) but I do agree that it should no longer be used on a live site. And yes, I'm using PDO and parametrised inputs.
     
    Ben Cousins, Mar 12, 2015 IP
  9. PoPSiCLe

    PoPSiCLe Illustrious Member

    Messages:
    4,623
    Likes Received:
    725
    Best Answers:
    152
    Trophy Points:
    470
    #9
    Why is that good? Using mysqli_ or PDO isn't any harder than learning mysql_ if you're an absolute beginner? If you have no idea how to do something, providing a broken tool for learning isn't a good idea. I realise that a LOT of tutorials online still use the mysql_-function, but that doesn't mean one shouldn't try to teach everyone a better way of doing, right from the start.
     
    PoPSiCLe, Mar 13, 2015 IP
    sarahk likes this.
  10. deathshadow

    deathshadow Acclaimed Member

    Messages:
    9,732
    Likes Received:
    1,998
    Best Answers:
    253
    Trophy Points:
    515
    #10
    No, it isn't. Learning buggy, insecure halfwit ways of doing things is NOT good from ANY perspective. PARTICULARLY for beginners because they'll end up having to unlearn all the bekaptah dumbass nonsense when it's time to put on the big boy pants.
     
    deathshadow, Mar 13, 2015 IP
    sarahk likes this.
  11. NetStar

    NetStar Notable Member

    Messages:
    2,471
    Likes Received:
    541
    Best Answers:
    21
    Trophy Points:
    245
    #11
    It's deprecated due to bad practice. Irrelevant of whether the depreciation occurred during the version you have installed or not, it's still bad practice and not secure.

    If you have a 9 year old version of PHP installed on your production or development machine you are probably following the path of bad practice on a lot of shit.......so cheers and carry on!
     
    NetStar, Mar 21, 2015 IP
  12. deathshadow

    deathshadow Acclaimed Member

    Messages:
    9,732
    Likes Received:
    1,998
    Best Answers:
    253
    Trophy Points:
    515
    #12
    Bad practice doesn't even COVER it. It's insecure. PERIOD.

    ... and I'm not just talking blindly pasting together values into the string, you can at least TRY to clean with mysql_real_escape_string -- aka -- damn_this_function_name_is_too_blasted_long. Even worse is that the connection is left in global scope.

    PHP is an interpreted language, it is WAY too easy to have a code elevation; as such leaving anything regarding the database security -- be it connection info (see Turdpress' dumbass idoitic halfwit "Let's put it in define!) or the open connection itself. Isolating scope of the connection lets you limit the places code elevations can do anything with it, therein limiting what/where can actually use it.

    The mere fact that you can call functions like mysql_query without a link identifier is mind-numbingly mind-blowingly STUPID if you know even the slightest about securing an interpreted language. Though certainly no more stupid than having several hundred code entry vectors and then putting the username, password and hostname into DEFINE...

    But to be fair, I'm the nutjob who does this:
    dbsettings.php
    <?php
    
    function dbSettings() {
    
    	if (!defined('dbSettingsSent')) {
    		define('dbSettingsSent', 1);
    		if (
    			(count($d = debug_backtrace()) == 3) &&
    			isset($d[2]) &&
    			($d[2]['function'] == 'main') && 
    			(str_replace(
    				'/dbSettings.php',
    				'/index.php',
    				str_replace('\\', '/', $d[0]['file'])
    			) == str_replace('\\', '/', $d[2]['file']))
    		)	return [ 
    		
    			// BEGIN USER EDITS 
    	
    			/*
    				*** SQL LOGIN INFORMATION ***
    				to use unix_socket comment out dbHost line and uncomment dbSock
    				or vice-versa
    			*/
    			'dbUser' => 'localRake',
    			'dbPass' => 'asdG3N8tCP82pzjm',
    			'dbName' => 'paladinCMS',
    	//		'dbHost' => 'localHost',
    			'dbSock' => '/opt/lampp/var/mysql/mysql.sock',
    			
    			/*
    				*** USER PASSWORD HASH AND SALT ***
    				You will likely want to create your own salt here
    			*/
    			'pwSalt' => 'STEAbarwe453afgbASDyHU',
    			'pwAlgo' => 'SHA256',
    			
    			/*
    				*** TABLE NAMES ***
    				The table prefix is prepended to all table names automatically. This
    				allows multiple copies of this CMS to be run from a single database
    
    				Handy when your hosting provider restricts the number of databases
    				you can use.
    			*/
    			'tablePrefix' => 'paladin_',
    			
    			/*
    				set installer password to 'false' to disable installer
    				Password is case sensitive!
    			*/
    			'installerPassword' => 'startItUp'
    		]; // DO NOT EDIT PAST THIS POINT!
    
    	}
    	die('Hacking attempt detected for dbSettings.php!');
    	
    }
    
    ?>
    Code (markup):
    So that only the main() function in my index.php can actually pull up this information... at least if I go ahead and block readfile.

    Though I still say that PHP should be unable to modify or read .php files with fopen or readfile, and only be able to include/require files that end in .php -- do you have ANY idea how many hacks/exploits would fall flat on their face with that simple change to how PHP works?
     
    Last edited: Mar 21, 2015
    deathshadow, Mar 21, 2015 IP
  13. NetStar

    NetStar Notable Member

    Messages:
    2,471
    Likes Received:
    541
    Best Answers:
    21
    Trophy Points:
    245
    #13
    Being insecure is why it's bad practice.........
     
    NetStar, Mar 22, 2015 IP