Have I successfully prevented an XSS attack with this code?

Question

So basically someone told me my site was vulnerable to an XSS attack, so I am making efforts to fix that. I was told that the htmlspecialchars method was a great way to prevent this.

I made this function

function _e($string){
    return htmlspecialchars($string, ENT_QUOTES, 'UTF-8');
}

And I am using it such as below

$userName = _e($_POST['userName']);
$Pass = _e($_POST['password']);

Questions:

1) Are XSS attacks only based on input, or do I need to do this on alerts and echo statements?

2) Have I successfully stopped XSS attacks in the page below?**

    <?php

    session_start();

    if(isset($_SESSION['user_id'])){
        header("Location: index.php");
    }

    function _e($string){
        return htmlspecialchars($string, ENT_QUOTES, 'UTF-8');
    }

    include '../includes/connection.php';
    $userName = _e($_POST['userName']);
    $Pass = _e($_POST['password']);



    if(!empty($userName) && !empty($Pass)){

        $sql = "SELECT * FROM Admins WHERE Username='$userName'";
        $sqlr = mysqli_query($connect,$sql);
        $sqlrow = $sqlr->fetch_assoc();
        $dbPass = $sqlrow['Password'];
        $hash = password_verify($Pass, $dbPass);

        if ($hash == 0){
            die("There was no password found matching what you have entered.");
        }else{
            $records = "SELECT * FROM Admins WHERE Username='$userName' AND Password='$dbPass' AND AdminLevel >=1";
            $results = mysqli_query($connect,$records);
            if ($results->num_rows == 1){
                $row = $results->fetch_assoc();
                $_SESSION['user_id'] = $row['ID'];
                $_SESSION['admin_level'] = $row['AdminLevel'];
                $_SESSION['user_name'] = $row['Username'];
                $easyName = $_SESSION['user_name'];
                $recordsS = "UPDATE `Admins` SET Status='1' WHERE Username='$userName'";
                $resultsS = mysqli_query($connect,$recordsS);
                header("Location: index.php");
            }else{
                $message = "Either you have entered the incorrect login information, or you account has not been approved yet.";
                echo "<script type='text/javascript'>alert('$message');</script>";
            }
        }
    }
?>
<!doctype html>
<html>
    <head>
        <meta charset="utf-8">
        <title>ServerSide Moderation Services</title>
        <link rel="stylesheet" href="../styles/mainStyles.css" type="text/css" />
        <link rel="stylesheet" href="../styles/loginFormStyle.css" type="text/css" />
        <meta name="viewport" content="width=device-width, initial-scale=1.0"/>
    </head>
    <body class="body">
        <?php include '../includes/header.php'; ?>

        <div class="mainContent">
            <div class="logRegArea">
                <article class="leftContent">
                    <header>
                        <h2 class="loginArea" style="text-align:center">Login Below:</h2>
                    </header>
                    <content>
                        <div id="login">
                            <form action="../pages/login.php" method="POST">
                                <input type="text" placeholder="Enter Your Username" name="userName">
                                <input type="password" placeholder="Enter Your Password" name="password">
                                <input type="submit">
                            </form>
                        </div>
                    </content>
                </article>
            </div>
        </div>
        <footer class="mainFooter">
            <p>This website was developed by ROBLOX user: <a href="https://www.roblox.com/users/8869935/profile" title="Made by: wattleman">wattleman</a></p>
        </footer>
    </body>
</html>

Please use PHP's hashing functions for passwords. Never store plain text passwords! — Jay Blanchard, Jul 23 '16 at 00:44

score 1 · Accepted Answer · edited May 23 '17 at 12:08

1

To answer your two questions directly:

1) If your message contains data from any source other than your code, sanitize it. Trust NOTHING.

2) Kind of. I can't see any output of your data anywhere, though.

There are two things you need to consider here.

1) By applying htmlspecialchars to the user-supplied data before insertion to the database, you have fixed most of the issues of XSS. However, it is generally better to insert the data "raw", and apply htmlspecialchars at the point when you echo it out to the page. That's because converting html entities makes sense for generating HTML, but nothing else. If you need to run searches, create exports etc., you'll probably find yourself needing to convert these entities back again.

2) You are open to SQL injection attacks. htmlspecialchars offers zero protection against that. You need to be using mysqli_real_escape_string or parametrised queries. More info about that here.

edited May 23 '17 at 12:08

Community

1
1

answered Jul 22 '16 at 22:58

rjdown

9,162
3
32
45

I appreciate the feed back. The only thing I ever take in is raw data. When I make an alert pop up, do I still need to use specialchars or since it's an echo with no input, is it fine? 2) I am working with prepared statements now. – Austin Jul 22 '16 at 23:01
If the message comes from a 100% trusted source with no chance of unescaped HTML entities, e.g. your hard-coded text, there's no reason to try to sanitise it – rjdown Jul 22 '16 at 23:14
I have completely changed the question. Do you mind going through it and helping me find the issue? – Austin Jul 22 '16 at 23:17
You should create a new question instead of changing an existing one. – rjdown Jul 22 '16 at 23:19
Well I had a timer, and I was trying to get the question answered quickly – Austin Jul 22 '16 at 23:23
Making a new question. – Austin Jul 22 '16 at 23:43

Have I successfully prevented an XSS attack with this code?

1 Answers1