code security

[PHKatz]

This code is for a comment system. Which is more secure against injection(xss) entities or chars?
[php]
if(isset($_POST[‘submit’])) { // Added not to display “please fill out all required fields”.
$name = htmlentities($name);
$email = htmlentities($email);
$website = htmlentities($website);
$message = htmlentities($message);
$time = time();
@ $fp = fopen(‘testcomments.log’,‘a’);
if (!$name || !$email|| !$website || !$message )
{
echo “Please fill out all required fields”;}
else
{
if(strlen($name) > 0 && strlen($message)> 0) {
if(filesize(‘testcomments.log’) > 0) {
$pre =’
’;
}
}

$outputstring = $pre. ‘

’.$name.’. ‘.date(‘F j Y \a\t h:i a’,$time).’

'.$message .'

'; @fwrite($fp, $outputstring, strlen($outputstring)); fclose($fp);

echo “”; // changed from Header( )
}
}

?>

   Name:
    Email:
Website:
Comments:

Comments:

<?php include "testcomments.log"; ?> [/php]

[astonecipher]

Have you looked at the docs for htmlentities? Why are you worried about the secure-ness of what is being passed? If it is intended to go into a database, I HIGHLY recommend going with PDO prepared statements, which render injections pointless as PDO self escapes.

[JimL]

Have you looked at the docs for htmlentities? Why are you worried about the secure-ness of what is being passed? If it is intended to go into a database, I HIGHLY recommend going with PDO prepared statements, which render injections pointless as PDO self escapes.

PDO/mysqli prepared statements does not automagically escape/quote values. They work kinda that way however by passing the statement (parameterized query) and the variables in separate. So the DB knows the variables passed in are just variables, and not part of the query.

In this post however OP does not mention anything about db/sql injection, so not sure why you bring it up.

OP:
htmlentities is better, I’d also expand it do declare which encoding you are using (UTF-8?). You could also consider using a library to clean data, like
http://htmlpurifier.org/comparison

[astonecipher]

PDO/mysqli prepared statements does not automatically escape/quote values. They work kinda that way however by passing the statement (parameterized query) and the variables in separate. So the DB knows the variables passed in are just variables, and not part of the query.

Which is essentially self escaping. You cannot gain access to the query through PDO, (if used correctly).

In this post however OP does not mention anything about db/sql injection, so not sure why you bring it up.

I bring it up because I misinterpreted the use of what the site is requesting…

[JimL]

Which is essentially self escaping.

I disagree, you get the same effect but it’s not using the same method at all. Especially since escaping has been a pain earlier it is nice to be able to distance ourselves from it and simply say it’s no longer neccessary

[astonecipher]

simply say it’s no longer neccessary

I will agree with that! So long as the implementation is used correctly anyway… I am sure someone still concates into mysqli and PDO if for no other reason than not knowing any better.

[Strider64]

I disagree, you get the same effect but it’s not using the same method at all. Especially since escaping has been a pain earlier it is nice to be able to distance ourselves from it and simply say it’s no longer neccessary

I’m not trying to beat a discussion to death, but both of you are kind of right. PDO in its initial configuration (depending on what PHP 5.x version you are using) is vulnerable. However, doing the following eliminates that problem, though I would note that nothing going on the Internet is 100 percent safe.

[php] $db_options = array(
// important! use actual prepared statements (default: emulate prepared statements)
PDO::ATTR_EMULATE_PREPARES => false
, PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION // throw exceptions on errors (default: stay silent)
, PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC // fetch associative arrays (default: mixed arrays)
); [/php]

[JimL]

Good catch. But as we were discussing prepared statements( not emulated prepared statements), I fail to see where we were “kind of wrong”

[PHKatz]

First, thanks to everyone for the help, expertise. Invaluable. To clarify, the comment system works and is without db. May include that in the future. But saw xss on one of my webpages, and wasn’t sure what the difference was, if any, as far as securing it. Once again, appreciated. 8)

[PHKatz]

Good catch. But as we were discussing prepared statements( not emulated prepared statements), I fail to see where we were “kind of wrong”

Is this the way it should be?

if(isset($_POST[‘submit’])) {
$name = htmlentities($name, ENT_QUOTES, “UTF-8”);
$email = htmlentities($email, ENT_QUOTES, “UTF-8”);
$website = htmlentities($website, ENT_QUOTES, “UTF-8”);
$message = htmlentities($message, ENT_QUOTES, “UTF-8”);
$time = time();
@ $fp = fopen(‘testcomments.log’,‘a’);
if (!$name || !$email|| !$website || !$message )
{
echo “Please fill out all required fields”;}
else
{
if(strlen($name) > 0 && strlen($message)> 0) {
if(filesize(‘testcomments.log’) > 0) {
$pre =’
’;
}
}

$outputstring = $pre. ‘

’.$name.’. ‘.date(‘F j Y \a\t h:i a’,$time).’

'.$message .'

'; @fwrite($fp, $outputstring, strlen($outputstring)); fclose($fp);

echo “”; // changed from Header( )
}
}

?>

Comments:

<?php include "testcomments.log"; ?>

[JimL]

Yup, id consider making a function for it just to make it easier.

[php]function escape ($unescaped) {
return htmlentities($unescaped, ENT_QUOTES, “UTF-8”);
}[/php]

Then you can very easily escape stuff with

[php]$name = escape($name);
$email = escape($email);
$website = escape($website); [/php]

Also remember that numbers can (should?) be escaped with type casting

[php]$start = ! empty($_POST [‘start’]) ? (int) $_POST [‘start’] : 0;[/php]

[PHKatz]

Yup, id consider making a function for it just to make it easier.

[php]function escape ($unescaped) {
return htmlentities($unescaped, ENT_QUOTES, “UTF-8”);
}[/php]

Then you can very easily escape stuff with

[php]$name = escape($name);
$email = escape($email);
$website = escape($website); [/php]

Also remember that numbers can (should?) be escaped with type casting

[php]$start = ! empty($_POST [‘start’]) ? (int) $_POST [‘start’] : 0;[/php]

$start = ! empty($_POST [‘start’]) ? (int) $_POST [‘start’] : 0;
function escape ($unescaped) {
return htmlentities($unescaped, ENT_QUOTES, “UTF-8”);
}
$name = escape($name);
$email = escape($email);
$website = escape($website);
$message = escape($message);
$time = time();
@ $fp = fopen(‘testcomments.log’,‘a’);
if (!$name || !$email|| !$website || !$message )
{
echo “Please fill out all required fields”;}
else
{
if(strlen($name) > 0 && strlen($message)> 0) {
if(filesize(‘testcomments.log’) > 0) {
$pre =’
’;
}
}

$outputstring = $pre. ‘

’.$name.’. ‘.date(‘F j Y \a\t h:i a’,$time).’

'.$message .'

'; @fwrite($fp, $outputstring, strlen($outputstring)); fclose($fp);

echo “”; // changed from Header( )
}
}

?>

[PHKatz]

Yup, id consider making a function for it just to make it easier.

[php]function escape ($unescaped) {
return htmlentities($unescaped, ENT_QUOTES, “UTF-8”);
}[/php]

Then you can very easily escape stuff with

[php]$name = escape($name);
$email = escape($email);
$website = escape($website); [/php]

Also remember that numbers can (should?) be escaped with type casting

[php]$start = ! empty($_POST [‘start’]) ? (int) $_POST [‘start’] : 0;[/php]

When I tested it after expanding it, the comment appeared, but name and date didn’t. Not sure what I missed.

[PHKatz]

When I tested it after expanding it, the comment appeared, but name and date didn’t. Not sure what I missed.

Modified code and received parse error on line 13. Not sure what it is.

<?php if(isset($_POST['submit'])) { // Added not to display "please fill out all required fields". $name = htmlentities($name, ENT_QUOTES, "UTF-8"); $email = htmlentities($email, ENT_QUOTES, "UTF-8"); $website = htmlentities($website, ENT_QUOTES, "UTF-8"); $message = htmlentities($message, ENT_QUOTES, "UTF-8"); $time = time(); function ($data) { return htmlentities($data, ENT_QUOTES, "UTF-8");}

if (!$name || !$email|| !$website || !$message )
{
echo “Please fill out all required fields”;}
else
{
if(strlen($name) > 0 && strlen($message)> 0) {
if(filesize(‘testcomments.log’) > 0) {
$pre =’
’;
}
}
$outputstring = $pre. ‘

’.$name.’. ‘.date(‘F j Y \a\t h:i a’,$time).’

'.$message .'

'; @fwrite($fp, $outputstring, strlen($outputstring)); fclose($fp);

echo “”; // changed from Header( )
}
}

?>

		 <input type="text" name="name" placeholder="Name">
		 <br /><br/> 
		 <input type="text" name="email" placeholder="Email">
		 <br /><br/> 
		 <input type="text" name="website" placeholder="Website">
		 <br /><br/>
		 <textarea name="message" cols="40" rows="4" placeholder="Comments"></textarea></br> 

Comments:

<?php include "testcomments.log"; ?>

[PHKatz]

Modified code and received parse error on line 13. Not sure what it is.

<?php if(isset($_POST['submit'])) { // Added not to display "please fill out all required fields". $name = htmlentities($name, ENT_QUOTES, "UTF-8"); $email = htmlentities($email, ENT_QUOTES, "UTF-8"); $website = htmlentities($website, ENT_QUOTES, "UTF-8"); $message = htmlentities($message, ENT_QUOTES, "UTF-8"); $time = time(); function ($data) { return htmlentities($data, ENT_QUOTES, "UTF-8");}

if (!$name || !$email|| !$website || !$message )
{
echo “Please fill out all required fields”;}
else
{
if(strlen($name) > 0 && strlen($message)> 0) {
if(filesize(‘testcomments.log’) > 0) {
$pre =’
’;
}
}
$outputstring = $pre. ‘

’.$name.’. ‘.date(‘F j Y \a\t h:i a’,$time).’

'.$message .'

'; @fwrite($fp, $outputstring, strlen($outputstring)); fclose($fp);

echo “”; // changed from Header( )
}
}

?>

		 <input type="text" name="name" placeholder="Name">
		 <br /><br/> 
		 <input type="text" name="email" placeholder="Email">
		 <br /><br/> 
		 <input type="text" name="website" placeholder="Website">
		 <br /><br/>
		 <textarea name="message" cols="40" rows="4" placeholder="Comments"></textarea></br> 

Comments:

<?php include "testcomments.log"; ?>

Is this correct?

<?php if(isset($_POST['submit'])) { // Added not to display "please fill out all required fields". $name = htmlentities($name, ENT_QUOTES, "UTF-8"); $email = htmlentities($email, ENT_QUOTES, "UTF-8"); $website = htmlentities($website, ENT_QUOTES, "UTF-8"); $message = htmlentities($message, ENT_QUOTES, "UTF-8"); $time = time(); function ($data) { $name = escape($name); $email = escape($email); $website = escape($website); return htmlentities($data, ENT_QUOTES, "UTF-8"); } if (!$name || !$email) { echo "Please fill out all required fields";} else { if(strlen($name) > 0 && strlen($message)> 0) { if(filesize('testcomments.log') > 0) { $pre ='
'; } } $outputstring = $pre. '

'.$name.'. '.date('F j Y \a\t h:i a',$time).'

'.$message .'

'; @fwrite($fp, $outputstring, strlen($outputstring)); fclose($fp);

echo “”; // changed from Header( )
}
}

?>

        <input type="text" name="name" placeholder="Name">
        <br /><br/> 
        <input type="text" name="email" placeholder="Email">
        <br /><br/> 
        <input type="text" name="website" placeholder="Website">
        <br /><br/>
        <textarea name="message" cols="40" rows="4" placeholder="Comments"></textarea></br> 

Comments:

<?php include "testcomments.log"; ?>

[PHKatz]

Is this correct?

<?php if(isset($_POST['submit'])) { // Added not to display "please fill out all required fields". $name = htmlentities($name, ENT_QUOTES, "UTF-8"); $email = htmlentities($email, ENT_QUOTES, "UTF-8"); $website = htmlentities($website, ENT_QUOTES, "UTF-8"); $message = htmlentities($message, ENT_QUOTES, "UTF-8"); $time = time(); function ($data) { $name = escape($name); $email = escape($email); $website = escape($website); return htmlentities($data, ENT_QUOTES, "UTF-8"); } if (!$name || !$email) { echo "Please fill out all required fields";} else { if(strlen($name) > 0 && strlen($message)> 0) { if(filesize('testcomments.log') > 0) { $pre ='
'; } } $outputstring = $pre. '

'.$name.'. '.date('F j Y \a\t h:i a',$time).'

'.$message .'

'; @fwrite($fp, $outputstring, strlen($outputstring)); fclose($fp);

echo “”; // changed from Header( )
}
}

?>

        <input type="text" name="name" placeholder="Name">
        <br /><br/> 
        <input type="text" name="email" placeholder="Email">
        <br /><br/> 
        <input type="text" name="website" placeholder="Website">
        <br /><br/>
        <textarea name="message" cols="40" rows="4" placeholder="Comments"></textarea></br> 

Comments:

<?php include "testcomments.log"; ?>

localhost listed line 5 as unidentified escape. Not sure where the error is?
$name = htmlentities($name, ENT_QUOTES, “UTF-8”);

[PHKatz]

Yup, id consider making a function for it just to make it easier.

[php]function escape ($unescaped) {
return htmlentities($unescaped, ENT_QUOTES, “UTF-8”);
}[/php]

Then you can very easily escape stuff with

[php]$name = escape($name);
$email = escape($email);
$website = escape($website); [/php]

Also remember that numbers can (should?) be escaped with type casting

[php]$start = ! empty($_POST [‘start’]) ? (int) $_POST [‘start’] : 0;[/php]

followed your suggestion.
localhost listed line 5 as unidentified escape. Not sure about error?
$name = htmlentities($name, ENT_QUOTES, “UTF-8”);

[JimL]

Post your code

[PHKatz]

Post your code

<?php if(isset($_POST['submit'])) { // Added not to display "please fill out all required fields". [b]$name = htmlentities($name, ENT_QUOTES, "UTF-8"); [/b] $email = htmlentities($email, ENT_QUOTES, "UTF-8"); $website = htmlentities($website, ENT_QUOTES, "UTF-8"); $message = htmlentities($message, ENT_QUOTES, "UTF-8"); $time = time(); function ($unescaped) { $name = escape($name); $email = escape($email); $website = escape($website); return htmlentities($unescaped, ENT_QUOTES, "UTF-8"); } if (!$name || !$email) { echo "Please fill out all required fields";} else { if(strlen($name) > 0 && strlen($message)> 0) { if(filesize('testcomments.log') > 0) { $pre ='
'; } } $outputstring = $pre. '

'.$name.'. '.date('F j Y \a\t h:i a',$time).'

'.$message .'

'; @fwrite($fp, $outputstring, strlen($outputstring)); fclose($fp);

echo “”; // changed from Header( )
}
}

?>

        <input type="text" name="name" placeholder="Name">
        <br /><br/> 
        <input type="text" name="email" placeholder="Email">
        <br /><br/> 
        <input type="text" name="website" placeholder="Website">
        <br /><br/>
        <textarea name="message" cols="40" rows="4" placeholder="Comments"></textarea></br> 

Comments:

<?php include "testcomments.log"; ?>

[JimL]

Where have you defined the escape function?