0

I have the following PHP function:

public function signup() {
        $mysql = mysqli_connect(HOSTNAME, USERNAME, PASSWORD, DATABASE);
        if (mysqli_connect_errno($mysql)) {
            $this->viewModel->set("pageTitle", "Signup");
            $this->viewModel->set("message", "There was an error connecting to the server.");
            return $this->viewModel;
        }
        if ($result = $mysql->query("SELECT id FROM mailinglist WHERE email='" . $this->email . "';")) {
            if ($result->num_rows == 0) {
                $mysql->query("INSERT INTO mailinglist (email) VALUES ('" . $this->email . "');");
                $this->viewModel->set("message", "Great! Thanks for signing up " . $this->email . ".");
            } else {
                $this->viewModel->set("message", "You are already signed up for updates!");
            }
        } else {
            $this->viewModel->set("message", "There was an error adding you the mailing list.");
        }
        $this->viewModel->set("pageTitle", "Signup");
        return $this->viewModel;
    }

Which runs fine and returns exactly what I want, however, if I try to use mysqli_real_escape_string() to my queries, it doesn't work. That is, the following code

public function signup() {
        $mysql = mysqli_connect(HOSTNAME, USERNAME, PASSWORD, DATABASE);
        if (mysqli_connect_errno($mysql)) {
            $this->viewModel->set("pageTitle", "Signup");
            $this->viewModel->set("message", "There was an error connecting to the server.");
            return $this->viewModel;
        }
        $query = $mysql->real_escape_string("SELECT id FROM mailinglist WHERE email='" . $this->email . "';");
        if ($result = $mysql->query($query)) {
            if ($result->num_rows == 0) {
                $query = $mysql->real_escape_string("INSERT INTO mailinglist (email) VALUES ('" . $this->email . "');");
                $mysql->query($query);
                $this->viewModel->set("message", "Great! Thanks for signing up " . $this->email . ".");
            } else {
                $this->viewModel->set("message", "You are already signed up for updates!");
            }
        } else {
            $this->viewModel->set("message", "There was an error adding you the mailing list.");
        }
        $this->viewModel->set("pageTitle", "Signup");
        return $this->viewModel;
    }

does not work. It is not a problem with the connection and I have tried using mysqli_real_escape_string() instead of $mysql->real_escape_string() but neither of them work. Can anyone see what is wrong with this code?

2
  • 10
    real_escape_string escapes data for use in a query; it doesn’t magically render a query you’ve already thrown arbitrary input into safe. But it doesn’t really matter what it does, because since you’re using MySQLi, you can use and should be using prepared statements. Commented Nov 5, 2013 at 3:47
  • Are there any PHP errors? Commented Nov 5, 2013 at 3:57

2 Answers 2

2

Don't do this, use prepared statements. They are safer and more reliable. You still need to sanitize the data for proper value and cross site scripting to name a few risks you'll still encounter. Escaping your data is one way of preventing SQL injection, but it's not full proof. Prepared statements tell the DB server to assume the incoming data is insecure and just accept it, and do not process it like a concatenated string. The database takes your parameters like they are variables for a statement rather than a part of the statement.

Here's how you can change yours to be a prepared statement:

$stmt=$mysql->prepare("SELECT id FROM mailinglist WHERE email=?");
$stmt->bind_param('s',$this->email);
$result=$stmt->execute();
if ($result) {
    if ($result->num_rows == 0) {
        $stmt=$mysql->prepare("INSERT INTO mailinglist (email) VALUES (?)");
        $stmt->bind_param('s', $this->email);
        $stmt->execute();
        $this->viewModel->set("message", "Great! Thanks for signing up " . $this->email . ".");
     } else {
        $this->viewModel->set("message", "You are already signed up for updates!");
     }
} else {
     $this->viewModel->set("message", "There was an error adding you the mailing list.");

}
Sign up to request clarification or add additional context in comments.

Comments

-1

You have to escape data not the whole query.

public function signup() {
        $mysql = mysqli_connect(HOSTNAME, USERNAME, PASSWORD, DATABASE);
        if (mysqli_connect_errno($mysql)) {
            $this->viewModel->set("pageTitle", "Signup");
            $this->viewModel->set("message", "There was an error connecting to the server.");
            return $this->viewModel;
        }
       $query = "SELECT id FROM mailinglist WHERE email='" .$mysql->real_escape_string( $this->email ). "'"
        if ($result = $mysql->query($query)) {
            if ($result->num_rows == 0) {
               $query = "INSERT INTO mailinglist (email) VALUES ('" . $mysql->real_escape_string($this->email) . "')";
                $mysql->query($query);
                $this->viewModel->set("message", "Great! Thanks for signing up " . $this->email . ".");
            } else {
                $this->viewModel->set("message", "You are already signed up for updates!");
            }
        } else {
            $this->viewModel->set("message", "There was an error adding you the mailing list.");
        }
        $this->viewModel->set("pageTitle", "Signup");
        return $this->viewModel;
    }

1 Comment

This whole answer is wrong. As you shouldn't escape "data" as well.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.