KPC
Kevin Powell - Community•2d ago
raw-power

Forms and Site Security Help

Hi everyone! I have most of my site set up with just HTML and CSS, I want to add a form to receive some information such as Name, Phone Number and Email Address and have that emailed to me. I have a php setup for that, but I'm worried that having a form may create a vulnerability in my site and allow it to be hacked (it's happened before). How are you all preventing injection attacks and other exploits?
46 Replies
Jochem
Jochem•2d ago
Prepared statements
raw-power
raw-powerOP•2d ago
<?php
if ($_SERVER["REQUEST_METHOD"] == "POST") {
    $name = htmlspecialchars($_POST['name']);
    $phone = htmlspecialchars($_POST['phone']);
    $email = htmlspecialchars($_POST['email']);
    $recaptchaResponse = $_POST['g-recaptcha-response'];

    // Verify reCAPTCHA
    $secretKey = 'YOUR_RECAPTCHA_SECRET_KEY';
    $response = file_get_contents("https://www.google.com/recaptcha/api/siteverify?secret=$secretKey&response=$recaptchaResponse");
    $responseKeys = json_decode($response, true);

    if (intval($responseKeys["success"]) !== 1) {
        die('Please complete the reCAPTCHA.');
    }

    // Validate email
    if (!filter_var($email, FILTER_VALIDATE_EMAIL)) {
        die('Invalid email format.');
    }

    // Prepare and send email
    $subject = 'New Contact Form Submission';
    $message = "Name: $name\nPhone: $phone\nEmail: $email";
    $headers = 'From: [email protected]' . "\r\n" .
               'Reply-To: [email protected]' . "\r\n" .
               'X-Mailer: PHP/' . phpversion();

    if (mail($to, $subject, $message, $headers)) {
        echo 'Message sent successfully!';
    } else {
        echo 'Failed to send message.';
    }
}
?>
<?php
if ($_SERVER["REQUEST_METHOD"] == "POST") {
    $name = htmlspecialchars($_POST['name']);
    $phone = htmlspecialchars($_POST['phone']);
    $email = htmlspecialchars($_POST['email']);
    $recaptchaResponse = $_POST['g-recaptcha-response'];

    // Verify reCAPTCHA
    $secretKey = 'YOUR_RECAPTCHA_SECRET_KEY';
    $response = file_get_contents("https://www.google.com/recaptcha/api/siteverify?secret=$secretKey&response=$recaptchaResponse");
    $responseKeys = json_decode($response, true);

    if (intval($responseKeys["success"]) !== 1) {
        die('Please complete the reCAPTCHA.');
    }

    // Validate email
    if (!filter_var($email, FILTER_VALIDATE_EMAIL)) {
        die('Invalid email format.');
    }

    // Prepare and send email
    $subject = 'New Contact Form Submission';
    $message = "Name: $name\nPhone: $phone\nEmail: $email";
    $headers = 'From: [email protected]' . "\r\n" .
               'Reply-To: [email protected]' . "\r\n" .
               'X-Mailer: PHP/' . phpversion();

    if (mail($to, $subject, $message, $headers)) {
        echo 'Message sent successfully!';
    } else {
        echo 'Failed to send message.';
    }
}
?>
Would this work or are there vulnerabilities still?
Jochem
Jochem•2d ago
oh, sorry, I assumed you meant SQL injection I'm not terribly up to date on the security concerns with sending mail through the mail() function, but that looks like a good start at least. Hopefully someone else can weigh in
raw-power
raw-powerOP•2d ago
thank you!
ἔρως
ἔρως•2d ago
it looks ok, but you dont implement any type of rate limiting from php you need to implement it from apache/nginx if you prefer
13eck
13eck•2d ago
This might be a good read. OWASP has some great stuff on security, as that's kinda their thing :p Also, see #How To Ask Good Questions to see how to properly format code blocks in Discord
ἔρως
ἔρως•2d ago
i always forget about owasp but seriously, i would consider not even sending an email i would just ram all the data into the database and then send a daily email with all the contacts or something also, you dont have any server-side validation or verificarion or any records at all in fact, this even sounds like you didnt even request consent to use user's data, as required by the gdpr some states are also slowly starting to have some data protection laws too, so, you should look into that if i were you, i would even avoid sending an email from your own server, as you will have to deal with an annoying amount of work, as would try to use sendgrid or other service that has a nice free tier
raw-power
raw-powerOP•2d ago
thank you, I've formatted the post I don't need gdpr where I am which is why it's not implemented. why do you say sending from my own server would be an annoying amount of work? oh didn't consider rate limiting, thank you, would throttling like this work? 
<?php
session_start();
$time_limit = 60; // Time limit in seconds

if (isset($_SESSION['last_submission'])) {
    $last_submission = $_SESSION['last_submission'];
    if (time() - $last_submission < $time_limit) {
        die('Please wait before submitting the form again.');
    }
}

$_SESSION['last_submission'] = time();

// Proceed with sending the email
mail($to, $subject, $message, $headers);
?>
<?php
session_start();
$time_limit = 60; // Time limit in seconds

if (isset($_SESSION['last_submission'])) {
    $last_submission = $_SESSION['last_submission'];
    if (time() - $last_submission < $time_limit) {
        die('Please wait before submitting the form again.');
    }
}

$_SESSION['last_submission'] = time();

// Proceed with sending the email
mail($to, $subject, $message, $headers);
?>
ἔρως
ἔρως•2d ago
no, i can just send from a "browser" that doesnt support cookies (or disable cookies in curl) you need to make it clear who the target audience is then, in the form and it is annoying because of all the configurations you have to do to try to avoid spoofing also, if some asshole decides to spam the living crap out of your server, and you send an email per request, your email box may get full and/or you get blacklisted
raw-power
raw-powerOP•2d ago
then how about using the database to store the session details such as: 
<?php
$ip_address = $_SERVER['REMOTE_ADDR'];
$time_limit = 60; // Time limit in seconds
$max_attempts = 5; // Maximum number of attempts allowed

// Connect to the database
$conn = new mysqli('hostname', 'username', 'password', 'database');

// Check if the IP address exists in the database
$query = "SELECT * FROM submissions WHERE ip_address = '$ip_address'";
$result = $conn->query($query);

if ($result->num_rows > 0) {
    $row = $result->fetch_assoc();
    $last_attempt = strtotime($row['last_attempt']);
    $attempts = $row['attempts'];

    if (time() - $last_attempt < $time_limit) {
        if ($attempts >= $max_attempts) {
            die('You have exceeded the maximum number of submissions. Please try again later.');
        } else {
            $attempts++;
            $query = "UPDATE submissions SET attempts = $attempts, last_attempt = NOW() WHERE ip_address = '$ip_address'";
            $conn->query($query);
        }
    } else {
        $query = "UPDATE submissions SET attempts = 1, last_attempt = NOW() WHERE ip_address = '$ip_address'";
        $conn->query($query);
    }
} else {
    $query = "INSERT INTO submissions (ip_address, attempts, last_attempt) VALUES ('$ip_address', 1, NOW())";
    $conn->query($query);
}

// Proceed with sending the email
mail($to, $subject, $message, $headers);
?>
<?php
$ip_address = $_SERVER['REMOTE_ADDR'];
$time_limit = 60; // Time limit in seconds
$max_attempts = 5; // Maximum number of attempts allowed

// Connect to the database
$conn = new mysqli('hostname', 'username', 'password', 'database');

// Check if the IP address exists in the database
$query = "SELECT * FROM submissions WHERE ip_address = '$ip_address'";
$result = $conn->query($query);

if ($result->num_rows > 0) {
    $row = $result->fetch_assoc();
    $last_attempt = strtotime($row['last_attempt']);
    $attempts = $row['attempts'];

    if (time() - $last_attempt < $time_limit) {
        if ($attempts >= $max_attempts) {
            die('You have exceeded the maximum number of submissions. Please try again later.');
        } else {
            $attempts++;
            $query = "UPDATE submissions SET attempts = $attempts, last_attempt = NOW() WHERE ip_address = '$ip_address'";
            $conn->query($query);
        }
    } else {
        $query = "UPDATE submissions SET attempts = 1, last_attempt = NOW() WHERE ip_address = '$ip_address'";
        $conn->query($query);
    }
} else {
    $query = "INSERT INTO submissions (ip_address, attempts, last_attempt) VALUES ('$ip_address', 1, NOW())";
    $conn->query($query);
}

// Proceed with sending the email
mail($to, $subject, $message, $headers);
?>
This plus ReCaptcha should do it? or can they still be bypassed?
ἔρως
ἔρως•2d ago
with recaptcha, it's possible to slow it down but you added a possible sql injection use prepared statements to insert stuff into the database also, you dont need to read and send the value to the database to increment by the way, remove the closing tag
raw-power
raw-powerOP•2d ago
which part do you see as a possible injection risk? none of this is coming from the form fields. not sure I understand what you meant by not needing to read and send to the database to increment, and yes, removing the closing tag for server efficiency, just keeping it there when doing snippet examples
ἔρως
ἔρως•2d ago
the ip address comes from external programs and if im not mistaken, it can be controlled in some scenarios
raw-power
raw-powerOP•2d ago
the ip address can be manipulated to inject sql code??? omg
ἔρως
ἔρως•2d ago
i think it is possible, yes
raw-power
raw-powerOP•2d ago
how is anyone implementing forms on their sites!?!
ἔρως
ἔρως•2d ago
painfully
raw-power
raw-powerOP•2d ago
seems so
ἔρως
ἔρως•2d ago
with prepared statements prepared statements have another advantage: mysql can cache the compiled query and subsequent queries are a lot faster
Jochem
Jochem•2d ago
it's very unlikely... but the reason you would still use prepared statements are twofold: 1) Forming the habbit. It's too important to ignore and good to just always use the right way 2) Just in case there's a PHP vulnerability where $_SERVER can be manipulated. It'd be much nicer to know that "huh, maybe the rate limiter won't work so well" rather than "in theory my entire DB is now vulnerable"
ἔρως
ἔρως•2d ago
if you provide the values directly, those are new queries and wont be cached
Jochem
Jochem•2d ago
basically, you're writing code with in the back of your mind that other parts of the stack might have vulnerabilities you didn't count on
ἔρως
ἔρως•2d ago
there's a lot of stuff that is user-controlled in the $_SERVER superglobal stuff you wouldnt expect
Jochem
Jochem•2d ago
pretty sure REMOTE_ADDR isn't, but it's still good practice
ἔρως
ἔρως•2d ago
it's passed by apache to php or nginx it's a good idea to be paranoid
raw-power
raw-powerOP•2d ago
so, like this? 
<?php
$ip_address = $_SERVER['REMOTE_ADDR'];
$time_limit = 60; // Time limit in seconds
$max_attempts = 5; // Maximum number of attempts allowed

// Connect to the database
$conn = new mysqli('hostname', 'username', 'password', 'database');

// Check if the IP address exists in the database
$query = "SELECT * FROM submissions WHERE ip_address = ?";
$stmt = $conn->prepare($query);
$stmt->bind_param('s', $ip_address);
$stmt->execute();
$result = $stmt->get_result();

if ($result->num_rows > 0) {
    $row = $result->fetch_assoc();
    $last_attempt = strtotime($row['last_attempt']);
    $attempts = $row['attempts'];

    if (time() - $last_attempt < $time_limit) {
        if ($attempts >= $max_attempts) {
            die('You have exceeded the maximum number of submissions. Please try again later.');
        } else {
            $attempts++;
            $query = "UPDATE submissions SET attempts = ?, last_attempt = NOW() WHERE ip_address = ?";
            $stmt = $conn->prepare($query);
            $stmt->bind_param('is', $attempts, $ip_address);
            $stmt->execute();
        }
    } else {
        $query = "UPDATE submissions SET attempts = 1, last_attempt = NOW() WHERE ip_address = ?";
        $stmt = $conn->prepare($query);
        $stmt->bind_param('s', $ip_address);
        $stmt->execute();
    }
} else {
    $query = "INSERT INTO submissions (ip_address, attempts, last_attempt) VALUES (?, 1, NOW())";
    $stmt = $conn->prepare($query);
    $stmt->bind_param('s', $ip_address);
    $stmt->execute();
}

// Proceed with sending the email
mail($to, $subject, $message, $headers);
?>
<?php
$ip_address = $_SERVER['REMOTE_ADDR'];
$time_limit = 60; // Time limit in seconds
$max_attempts = 5; // Maximum number of attempts allowed

// Connect to the database
$conn = new mysqli('hostname', 'username', 'password', 'database');

// Check if the IP address exists in the database
$query = "SELECT * FROM submissions WHERE ip_address = ?";
$stmt = $conn->prepare($query);
$stmt->bind_param('s', $ip_address);
$stmt->execute();
$result = $stmt->get_result();

if ($result->num_rows > 0) {
    $row = $result->fetch_assoc();
    $last_attempt = strtotime($row['last_attempt']);
    $attempts = $row['attempts'];

    if (time() - $last_attempt < $time_limit) {
        if ($attempts >= $max_attempts) {
            die('You have exceeded the maximum number of submissions. Please try again later.');
        } else {
            $attempts++;
            $query = "UPDATE submissions SET attempts = ?, last_attempt = NOW() WHERE ip_address = ?";
            $stmt = $conn->prepare($query);
            $stmt->bind_param('is', $attempts, $ip_address);
            $stmt->execute();
        }
    } else {
        $query = "UPDATE submissions SET attempts = 1, last_attempt = NOW() WHERE ip_address = ?";
        $stmt = $conn->prepare($query);
        $stmt->bind_param('s', $ip_address);
        $stmt->execute();
    }
} else {
    $query = "INSERT INTO submissions (ip_address, attempts, last_attempt) VALUES (?, 1, NOW())";
    $stmt = $conn->prepare($query);
    $stmt->bind_param('s', $ip_address);
    $stmt->execute();
}

// Proceed with sending the email
mail($to, $subject, $message, $headers);
?>
ἔρως
ἔρως•2d ago
i just searched a bit and the value can be controlled if you are behind a proxy not a lot of control, but still now, you have a concurrency problem
raw-power
raw-powerOP•2d ago
fml
ἔρως
ἔρως•2d ago
if i send 2 requests super quickly, i can submit twice and count as 1 you are reading the number of attempts and then storing it into the database instead, just update in the database let mysql do the increment for you you can read the value again after, if you want, just so you have the updated number of attempts and then you check again
raw-power
raw-powerOP•2d ago
this?? 
<?php
$ip_address = $_SERVER['REMOTE_ADDR'];
$time_limit = 60; // Time limit in seconds
$max_attempts = 5; // Maximum number of attempts allowed

// Connect to the database
$conn = new mysqli('hostname', 'username', 'password', 'database');

// Start a transaction
$conn->begin_transaction(MYSQLI_TRANS_START_READ_WRITE);

// Lock the table to prevent concurrent access
$conn->query("LOCK TABLES submissions WRITE");

// Check if the IP address exists in the database
$query = "SELECT * FROM submissions WHERE ip_address = ?";
$stmt = $conn->prepare($query);
$stmt->bind_param('s', $ip_address);
$stmt->execute();
$result = $stmt->get_result();

if ($result->num_rows > 0) {
    $row = $result->fetch_assoc();
    $last_attempt = strtotime($row['last_attempt']);
    $attempts = $row['attempts'];

    if (time() - $last_attempt < $time_limit) {
        if ($attempts >= $max_attempts) {
            // Unlock the table and rollback the transaction
            $conn->query("UNLOCK TABLES");
            $conn->rollback();
            die('You have exceeded the maximum number of submissions. Please try again later.');
        } else {
            $attempts++;
            $query = "UPDATE submissions SET attempts = ?, last_attempt = NOW() WHERE ip_address = ?";
            $stmt = $conn->prepare($query);
            $stmt->bind_param('is', $attempts, $ip_address);
            $stmt->execute();
        }
    } else {
        $query = "UPDATE submissions SET attempts = 1, last_attempt = NOW() WHERE ip_address = ?";
        $stmt = $conn->prepare($query);
        $stmt->bind_param('s', $ip_address);
        $stmt->execute();
    }
} else {
    $query = "INSERT INTO submissions (ip_address, attempts, last_attempt) VALUES (?, 1, NOW())";
    $stmt = $conn->prepare($query);
    $stmt->bind_param('s', $ip_address);
    $stmt->execute();
}

// Unlock and commit 
$conn->query("UNLOCK TABLES");
$conn->commit();

// sending email
mail($to, $subject, $message, $headers);
?>
<?php
$ip_address = $_SERVER['REMOTE_ADDR'];
$time_limit = 60; // Time limit in seconds
$max_attempts = 5; // Maximum number of attempts allowed

// Connect to the database
$conn = new mysqli('hostname', 'username', 'password', 'database');

// Start a transaction
$conn->begin_transaction(MYSQLI_TRANS_START_READ_WRITE);

// Lock the table to prevent concurrent access
$conn->query("LOCK TABLES submissions WRITE");

// Check if the IP address exists in the database
$query = "SELECT * FROM submissions WHERE ip_address = ?";
$stmt = $conn->prepare($query);
$stmt->bind_param('s', $ip_address);
$stmt->execute();
$result = $stmt->get_result();

if ($result->num_rows > 0) {
    $row = $result->fetch_assoc();
    $last_attempt = strtotime($row['last_attempt']);
    $attempts = $row['attempts'];

    if (time() - $last_attempt < $time_limit) {
        if ($attempts >= $max_attempts) {
            // Unlock the table and rollback the transaction
            $conn->query("UNLOCK TABLES");
            $conn->rollback();
            die('You have exceeded the maximum number of submissions. Please try again later.');
        } else {
            $attempts++;
            $query = "UPDATE submissions SET attempts = ?, last_attempt = NOW() WHERE ip_address = ?";
            $stmt = $conn->prepare($query);
            $stmt->bind_param('is', $attempts, $ip_address);
            $stmt->execute();
        }
    } else {
        $query = "UPDATE submissions SET attempts = 1, last_attempt = NOW() WHERE ip_address = ?";
        $stmt = $conn->prepare($query);
        $stmt->bind_param('s', $ip_address);
        $stmt->execute();
    }
} else {
    $query = "INSERT INTO submissions (ip_address, attempts, last_attempt) VALUES (?, 1, NOW())";
    $stmt = $conn->prepare($query);
    $stmt->bind_param('s', $ip_address);
    $stmt->execute();
}

// Unlock and commit 
$conn->query("UNLOCK TABLES");
$conn->commit();

// sending email
mail($to, $subject, $message, $headers);
?>
ἔρως
ἔρως•2d ago
you are setting the count to 1 all the time wait, no, im wrong
raw-power
raw-powerOP•2d ago
only once the time limit is exceeded no?
ἔρως
ἔρως•2d ago
yes
raw-power
raw-powerOP•2d ago
phew
ἔρως
ἔρως•2d ago
also, remove the closing tag
raw-power
raw-powerOP•2d ago
so you think this, plus the prepared statements in my higher up example for the form itself, plus the recaptcha should be sufficient?
ἔρως
ἔρως•2d ago
should slow down the spam, yes
raw-power
raw-powerOP•2d ago
and the risk of exploiting the server (putting files, executing, manipulating the db)? I don't mind getting some spam, it's just I don't want my whole site deleted and replaced by a bunch of random pages (like had happened before)
ἔρως
ἔρως•2d ago
well, the way that file uploads work, you WILL have files put in your server so, you have to configure php to only accept files of up to a certain size or as small as possible, if you dont need file uploads
raw-power
raw-powerOP•2d ago
huh? it's just a contact form, three fields, Name, Email and Phone. They can use the form to upload files???
ἔρως
ἔρως•2d ago
yes, with a post request thats a part of how web servers work the file is deleted once the request finishes, but still if you have configured to allow uploads of 100mb, and your server has 10gb of space, you just need 100 bots with intentionally slow speeds to absolutely murder your server php is configured to 2mb, which would require 5000 bots sending 2mb at snails pace a much more unfeasable ddos vector with that configuration vs a custom 100mb
raw-power
raw-powerOP•2d ago
but if I'm the only one uploading files to the server through my backend, then I could just turn uploads off like this? 
<?php
// Check if a file was uploaded
if (isset($_FILES['file'])) {
    // Get the MIME type of the uploaded file
    $file_type = mime_content_type($_FILES['file']['tmp_name']);
    
    // Define an empty array of allowed types
    $allowed_types = [];
    
    // Check if the file type is in the allowed types array
    if (!in_array($file_type, $allowed_types)) {
        die('File uploads are not allowed.');
    }
    
    // check the file extension
    $file_extension = pathinfo($_FILES['file']['name'], PATHINFO_EXTENSION);
    $allowed_extensions = [];
    
    if (!in_array($file_extension, $allowed_extensions)) {
        die('File uploads are not allowed.');
    }
    
    // If the file type and extension are allowed, proceed with the upload
    // (In this case, no file types or extensions are allowed, so this code will never be reached)
}
?>
<?php
// Check if a file was uploaded
if (isset($_FILES['file'])) {
    // Get the MIME type of the uploaded file
    $file_type = mime_content_type($_FILES['file']['tmp_name']);
    
    // Define an empty array of allowed types
    $allowed_types = [];
    
    // Check if the file type is in the allowed types array
    if (!in_array($file_type, $allowed_types)) {
        die('File uploads are not allowed.');
    }
    
    // check the file extension
    $file_extension = pathinfo($_FILES['file']['name'], PATHINFO_EXTENSION);
    $allowed_extensions = [];
    
    if (!in_array($file_extension, $allowed_extensions)) {
        die('File uploads are not allowed.');
    }
    
    // If the file type and extension are allowed, proceed with the upload
    // (In this case, no file types or extensions are allowed, so this code will never be reached)
}
?>
ἔρως
ἔρως•2d ago
at that point, the file is already in the server
raw-power
raw-powerOP•2d ago
😭
ἔρως
ἔρως•this hour
if your server uses apache, you might want to use a .htaccess file to set some php flags this way, you can disable user uploads
raw-power
raw-powerOP•11h ago
thank you, yes it does

Did you find this page helpful?