-1

I've working on some security stuff, and php is giving me some odd behaviour. Could you help me figure out what's going on? :)

So, I have an array of inputs, like so

$first_name = "<script>alert();</script>";
$middle_name = 'Robert';
$last_name = 'Smith';
$username = 'testusername1';
$email = "testemail@mail.com";
$password = 'banana1';

and I am testing them for XSS, using htmlspecialchars, like this.

$first_name = htmlspecialchars($first_name, ENT_QUOTES, 'UTF-8');

Which works just fine to stop the script in the $first_name running.

However, paste this code into a foreach loop, and the javascript alert runs.

Here is my current foreach loop (not working properly)

$strings = 
array($first_name,$middle_name,$last_name,$username,$email,$password);

foreach($strings as $string) {
    $string = htmlspecialchars($string, ENT_QUOTES, 'UTF-8');
}

I'm not sure what I'm missing here. I guess it's something to do with assigning the converted string back into the array? But that sounds so confusing it just doesn't feel like the right answer. Idk.

Thank you for your help.

Andrew

5 Answers5

1

Looks like you're trying to make a foreach loop by reference, You need a & to make your foreach data changes persist outside of that loop...

foreach($strings as $string) {

Should be:

foreach($strings as &$string) {

As kindly pointed out by @CBroe we must use unset to free the variable used in the foreach loop... Therefore your full solution will become:

$strings = 
array($first_name,$middle_name,$last_name,$username,$email,$password);

foreach($strings as &$string) {
    $string = htmlspecialchars($string, ENT_QUOTES, 'UTF-8');
}     

unset($string);
IsThisJavascript
  • 1,730
  • 2
  • 15
  • 24
  • 1
    You should edit this example so that it also shows where to properly use `unset`, otherwise this is dangerous. – CBroe Jun 07 '18 at 10:59
  • 1
    @CBroe Thanks for the comment I've made the requested change – IsThisJavascript Jun 07 '18 at 11:02
  • Thanks for your help, but this actually isn't working. Would you be so kind as to help me out if I dump out my code to a pastebin? Mabye I do just have to do it the long way... https://pastebin.com/fyBzY0kG – andyswebportfolio Jun 07 '18 at 11:14
  • @aerotortoise Hey, sorry for the late reply I was on my lunch break. I just ran your code on my server and it worked fine however, your comments actually comment out the code... On your final comment lines that looks something like... `/*****` make sure you finish the comment off with a `/` so it becomes something like... `/******/` that's the only issue I see. – IsThisJavascript Jun 07 '18 at 11:40
  • Ah well, guess I'll have to do it the long way or upgrade my system. Have a good day. – andyswebportfolio Jun 07 '18 at 11:47
  • I don't think the system is the issue, it may be how you are outputting the data. Would you mind sharing that part of the code? Or if possible, your full php script... Every single solution provided here would give you your expected output. – IsThisJavascript Jun 07 '18 at 12:47
0

You cannot reassign an array element like that. You need to do it with the element's index.

foreach($strings as $index=>$string) {
    $strings[$index] = htmlspecialchars($string, ENT_QUOTES, 'UTF-8');
}
Mr Glass
  • 1,178
  • 1
  • 6
  • 12
0

You could use the following method without using references:

foreach($strings as $key => $string) {
    $strings[$key] = htmlspecialchars($string, ENT_QUOTES, 'UTF-8');
}
Liam G
  • 589
  • 3
  • 16
0

there is a problem in foreach, maybe the htmlspecialchars function will be run correctly. please check following article.

https://stackoverflow.com/questions/10057671/how-does-php-foreach-actually-work
Eric Lee
  • 56
  • 7
0

foreach works on a copy of the the array, no changes will persist.

// your array
$strings = array(
    'first_name'    => $firstname,
    'middle_name'   => $middle_name,
    'last_name'     => $last_name,
    'user_name'     => $username,
    'email'         => $email,
    'password'      => $password
);

// filtered: the array which contains your escaped data.
$filtered = array_map(function($string) { 
                  return htmlspecialchars($string, ENT_QUOTES, 'UTF-8'); 
            }, $strings);

// to access your filtered data:
// $filtered['first_name'] contains the filtered first name
// $filtered['middle_name'] contains the filtered middle name
// and so on ...
xanadev
  • 713
  • 9
  • 24
  • @aerotortoise i've already tested the code and it's working, i have updated the answer to use associative arrays, which are much more easy to read and access. try it :) – xanadev Jun 07 '18 at 12:46
  • Thanks for the help - It's still not working on this end, I guess I'll just have to do it the long way... – andyswebportfolio Jun 07 '18 at 12:55