0

I am trying to call a javascript function from php. The function takes one argument. This is the call:

<?php

function welcomeBackMessage($user, $password){
  echo "<h1>BACK: " . $user . "pwd " . $password . "</h1>";
  echo "<script> welcomeBack({$user})</script>";

}
?>

This is the JS function; declared before the php functions calls it:

<script>
    //Called form php to welcome back a guest
function welcomeBack(user){

  print("JS WELCOME BACK CALLED");
  var  login = document.getElementById(loginbutton);
  login.parentNode.removeChild(login);
}

The JS function never executes, and debuting the console I get the following:

<h1>BACK: guest2pwd 1234</h1><script> welcomeBack(guest2)</script>
Can't find variable: guest1.

The idea is to pass the variable to a JS function so I get update a DOM element with variable.

Any input appreciated.

dancingbush
  • 2,131
  • 5
  • 30
  • 66

1 Answers1

2

You need quotes around the string.

echo "<script> welcomeBack('{$user}')</script>";
Barmar
  • 741,623
  • 53
  • 500
  • 612
  • This is an XSS attack waiting to happen. – CherryDT Apr 24 '20 at 22:54
  • 1
    That depends on where `$user` comes from. – Barmar Apr 24 '20 at 22:56
  • ...which you can't know, and even if the OP thinks they know, it may change tomorrow by an unrelated change elsewhere in the code. You should always take care of avoiding escaping issues at the _output_, not at the input. Even if the admin personally creates the users, you wouldn't want the name `O'Neill` to break the page either, do you? The right way here would be `""` – CherryDT Apr 24 '20 at 23:01
  • Thanks for feedback, just to note the is a proof of concept web page, not for publishing – dancingbush Apr 24 '20 at 23:06
  • @CherryDT This is not arbitrary, user-specified data. It's a username, which usually has restricted syntax. – Barmar Apr 24 '20 at 23:17
  • That's not necessarily the case, it may also be the user's _display name_. And even if were a username, there wouldn't be any guarantee that it matches _these_ restrictions (for example, why do you expect `O'Neill` from being disallowed?). And there shouldn't be, because it's not logical that your restrictions for a value need to be kept in sync with the _union_ of all restrictions of places where you happen to _use_ that value... – CherryDT Apr 24 '20 at 23:23
  • Plus, it stays an attack vector, even if not immediately usable. Because now something like forgetting to check the username restrictions on the more obscure "change username" page, even though they are enforced on "register", which in itself would be relatively harmless because username restrictions nowadays usually exist to make identification easier for humans and not for machines (to avoid having `Admïn` impersonating `Admin`, etc.), becomes a door to an XSS vulnerability. Security should be followed through, on every step, not just on one. Otherwise you make hackers' lives a joy. – CherryDT Apr 24 '20 at 23:27