7 Replies - 627 Views - Last Post: 15 September 2013 - 11:33 AM Rate Topic: -----

#1 mutago234  Icon User is offline

  • D.I.C Head

Reputation: 0
  • View blog
  • Posts: 176
  • Joined: 08-September 13

String conversion before htmlentities

Posted 11 September 2013 - 07:13 PM

I makes string conversion before htmlentities the echoed data. My question is, is this secured against XSS attack.


// DATABASE connection


$result = $db->prepare("select cont_id,title,dt,name,short_dtl
from newscontent,content_cat where newscontent.cat_id=content_cat.cat_id
and newscontent.cat_id=$cat_id order by dt limit 50");
$result->execute(array( ));

echo "<table width='50%' border='0' cellspacing='1' cellpadding='0'>";
 while($nt = $result->fetch())

                 {

// string conversion and htmlentities before echoing

$myvar = htmlentities("$nt[cont_id]", ENT_QUOTES, 'UTF-8');
$myvar2 = htmlentities("$nt[title]", ENT_QUOTES, 'UTF-8');
$myvar3 = htmlentities("$nt[dt]", ENT_QUOTES, 'UTF-8');
$myvar4 = htmlentities("$nt[name]", ENT_QUOTES, 'UTF-8');


 echo "<tr bgcolor='#f1f1f1'><td> <b><a href=newscont-dtl.php?cont_id=$myvar>$myvar2</a></b>: $myvar3 $myvar4 </td>
<tr><td>$nt[short_dtl]</td><td></tr>
";
 
 


Is This A Good Question/Topic? 0
  • +

Replies To: String conversion before htmlentities

#2 Atli  Icon User is offline

  • D.I.C Lover
  • member icon

Reputation: 3730
  • View blog
  • Posts: 6,017
  • Joined: 08-June 10

Re: String conversion before htmlentities

Posted 11 September 2013 - 09:08 PM

You are putting the short_dtl field directly into the HTML you print at the bottom. I can't tell what that field is, or what kind of value it holds, so I can't tell if it's a XSS risk. If it's a number, then it should be fine, but if it's a string type then it is a threat.

There are also a few things in that code that you should look at:

  • In your SQL query, you seem to be using PDO's prepare method, but you are still injecting variables into the query string like people used to do before PDO or MySQLi introduced the ability to use parametrized statements with MySQL. You should be using parameter placeholders in the SQL and binding the values to them, or just passing the values in through the array in the execute call. - I suggest you read through PDO's manual entry on Prepared Statements to see how to do this properly.

  • Unless you are actually injecting variables into a larger string, you don't have to enclose a variable in a string to get it's value.
    // The string injection here is pointless.
    echo "$nt[title]";
    
    // It's enough to just reference the variable.
    echo $nt['title'];
    
    


  • Assuming the cont_id field is a numeric ID, stored as an integer or another number type in MySQL, there is no need to run that through the HTML entity encoding functions. The fact that it's a number automatically excludes it as a XSS threat. If you don't trust it, the simpler solution to ensure it's safety would be to type-cast it to the expected type, like: (int)$nt["cont_id"]. That would either return the real number, or 0 if it's invalid.

  • $nt, $myvar, $myvar2, $myvar3 and $myvar4 are crappy variable names. The less readable your code is, the worse it is. Readability is extremely important for coders, seeing as we tend to read code a lot more than write it. - It's always better to spend a couple of seconds to invent a descriptive name for variables, rather than just choose some random, useless names like that.

    Think of it like this. If you read the final HTML echo statement, you have no clue what each of the variables you are injecting will print into the HTML. Not without having to going up and finding where the variables are defined and examining how they are set. That kind of detouring when reading code costs you time and it also tends to be very annoying. It shouldn't be necessary.

Was This Post Helpful? 1
  • +
  • -

#3 mutago234  Icon User is offline

  • D.I.C Head

Reputation: 0
  • View blog
  • Posts: 176
  • Joined: 08-September 13

Re: String conversion before htmlentities

Posted 12 September 2013 - 12:13 AM

it displays the error below when run it as follows
Warning: PDOStatement::execute() [pdostatement.execute]: SQLSTATE[HY093]: Invalid parameter number: number of bound variables does not match number of tokens in C:\xampp\htdocs\copytimeline\timeline\newscat.php on line 89


$cat_id=strip_tags($_GET['cat_id']);

$result = $db->prepare("select cont_id,title,dt,name,short_dtl 
from newscontent,content_cat where newscontent.cat_id = ?
and newscontent.cat_id = ? order by dt limit 50");
$result->execute(array(
':newscontent.cat_id' => 'content_cat.cat_id',
':newscontent.cat_id' => $cat_id));



Was This Post Helpful? 0
  • +
  • -

#4 Atli  Icon User is offline

  • D.I.C Lover
  • member icon

Reputation: 3730
  • View blog
  • Posts: 6,017
  • Joined: 08-June 10

Re: String conversion before htmlentities

Posted 12 September 2013 - 12:29 AM

You seem to be misunderstanding how the placeholders work. The docs for the execute and bindValue functions explain exactly how they work, and have a few examples of the correct usage.

Essentially, there are two ways: named placeholders and question mark placeholders.
// Question mark:
$sql = "SELECT stuff FROM tbl WHERE id = ?";
$stmt = $pdo->prepare($sql);
$stmt->execute(array($_GET["id"]));

// Named:
$sql = "SELECT stuff FROM tbl WHERE id = :id";
$stmt = $pdo->prepare($sql);
$stmt->execute(array(":id" => $_GET["id"]));


With question mark placeholders you just provide a list of parameters to the execute method, where each item in the list matches a single question mark; and with the named placeholders you do the same, except you provide the placeholder name as the key in the array you give to the execute method.


Also, using the strip_tags function like that on the $_GET['cat_id'] makes no sense. You aren't printing it into a HTML page, so there is no risk of XSS injection, and it's not like HTML tags matter as far as SQL Injection goes. It's a fairly pointless thing to do. - It's not enough to just know what functions can be used for security, it's also important to know when they should be used.
Was This Post Helpful? 0
  • +
  • -

#5 mutago234  Icon User is offline

  • D.I.C Head

Reputation: 0
  • View blog
  • Posts: 176
  • Joined: 08-September 13

Re: String conversion before htmlentities

Posted 12 September 2013 - 01:07 AM

Even with name parameter as shown below, the same error occurs. Is it possible to do string conversion before querying the database if may suggest
here is the error

Warning: PDOStatement::execute() [pdostatement.execute]: SQLSTATE[HY093]: Invalid parameter number: number of bound variables does not match number of tokens in C:\xampp\htdocs\copytimeline\timeline\newscat.php on line 89


$result = $db->prepare("select cont_id,title,dt,name,short_dtl 
from newscontent,content_cat where newscontent.cat_id = :newscontent.cat_id
and newscontent.cat_id = :$cat_id order by dt limit 50");
$result->execute(array(
':newscontent.cat_id' => 'content_cat.cat_id',
':newscontent.cat_id' => $cat_id));




Was This Post Helpful? 0
  • +
  • -

#6 Atli  Icon User is offline

  • D.I.C Lover
  • member icon

Reputation: 3730
  • View blog
  • Posts: 6,017
  • Joined: 08-June 10

Re: String conversion before htmlentities

Posted 12 September 2013 - 03:29 AM

That code is far from being like that suggested by the doc entries, or even the examples I posted.

First of all, :newscontent.cat_id is to complex to be a named parameter. They are just simple placeholders that should be no more complex than PHP variable names. You are overcomplicating things by trying to mimic the table.field syntax.

Second, the :$cat_id placeholder is problematic because $cat_id is a PHP variable, and because your query string is in double-quotes, PHP will replace it with it's value. Meaning you'll end up with :123 as the placeholder, assuming $cat_id has 123 as it's value.

And third, the array you pass to the execute() call has a couple of problems as well: What is the first element suppose to be; why do you put the field name as the value? And why do you use the same key for both elements in the array?

Try to follow the advice and the examples in the doc entries I linked to.
Was This Post Helpful? 1
  • +
  • -

#7 mutago234  Icon User is offline

  • D.I.C Head

Reputation: 0
  • View blog
  • Posts: 176
  • Joined: 08-September 13

Re: String conversion before htmlentities

Posted 12 September 2013 - 10:25 AM

You are too much ATLI and with this, it works like charm

thank you


$result = $db->prepare('select cont_id,title,dt,name,short_dtl
from newscontent,content_cat where newscontent.cat_id = ?  and content_cat.cat_id = ? order by dt limit 50');
 

$result->bindParam(1, $cat_id, PDO::PARAM_INT);
$result->bindParam(2, $cat_id1, PDO::PARAM_INT);
$result->execute();



Was This Post Helpful? 0
  • +
  • -

#8 Dormilich  Icon User is offline

  • 痛覚残留
  • member icon

Reputation: 3572
  • View blog
  • Posts: 10,414
  • Joined: 08-June 10

Re: String conversion before htmlentities

Posted 15 September 2013 - 11:33 AM

unless you want to execute that statement multiple times, I recomment to use bindValue() instead of bindParam().
Was This Post Helpful? 0
  • +
  • -

Page 1 of 1