2
\$\begingroup\$

I have written code to send a simple email submission form to a sql table so that I can manage the data. I would like feedback on whether or not the code that I have written is efficient and secure. I have a config file that defines constants for database credentials.

HTML FORM

<form method="post" action="index.php">
    <input class="bt-email" type="email" name="email" placeholder="enter email address">
    <input class="bt-submit" type="submit" value="let's get moving!">
</form>

PHP Database Code File

try {
    $db = new PDO("mysql:host=" . DB_HOST . ";dbname=" . DB_NAME . ";port=" . DB_PORT,DB_USER,DB_PASS);
    $db->setAttribute(PDO::ATTR_ERRMODE,PDO::ERRMODE_EXCEPTION);
    $db->exec("SET NAMES 'utf8'");
} catch (Exception $e) {
    echo "Could not connect to the database.";
    exit;
}

PHP/SQL Submission Code

<?php
require_once("inc/config.php");
if ($_SERVER["REQUEST_METHOD"] == "POST") {
    $email = $_POST["email"];
    require(ROOT_PATH . "inc/database.php");
    try {
        $db->exec("REPLACE INTO launch_email VALUES ('$email')");
    } catch (Exception $e) {
        echo "Data could not be submitted to the database.";
        exit;
    }
    header("Location: index.php?status=thanks");
    exit;
}
?>
\$\endgroup\$
3
\$\begingroup\$

You could possibly move the database include before the server checks and sets the $_POST variable, as it would not be needed if the database is unable to connect.

I notice there is no sanitization of the email variable. This may be helpful.

\$\endgroup\$
1
\$\begingroup\$

I will extend mulquin's short post.

Sanitizing your POST will enable you to make sure there are no SQL injection. Since you are using PDO - you should use prepare() function reather than exec(). exec doesn't escape your query. (as shown in the link provided by mulquin)

Furthermore - you are not checking if the email is indeed an email or not what happens if its not a real email address (ie the format is not john@doe.com) - you will be storing gebrish at this point. So a bit of validation so that not only spammers won't just enter anything.

\$\endgroup\$
  • \$\begingroup\$ yeah i plan on adding some server-side validation, i just wanted to see if my code was efficient and secure from a sql standpoint. thanks! \$\endgroup\$ – Ryan Salmons Apr 11 '14 at 3:06

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.