3
\$\begingroup\$

Background

The basics of this project are already covered here.

I received the suggestions to:

  • not create the exercise with JavaScript.
  • use pictures instead of plaintext.

I tried to implement the suggestions like this:

The code

Example-form:

<form method="POST" action="../php/send.php" class='input'>
    <label>Your Name:</label><br>
    <input type="text" name="myName" placeholder="Name" required/><br><br>
    <label>Your Email:</label><br>
    <input type="text" name="myEmail" placeholder="E-Mail" required/><br><br>
    
    <!-- Honeypott -->
    <input type="text" id="website" name="website"/>
    <label>Message:</label><br>
    <textarea rows="8" name="myMessage" style='width: 100%; resize: none; border: 1px solid Gray; border-radius: 4px; box-sizing: border-box; padding: 10px 10px;' placeholder="Message" required></textarea><br><br>
    <input id='exerciseText' name='exerciseText', style='display: none;' value='
        <?php
        include '../php/randomExercise.php';
        $var = randText();
        echo $var;
        ?>'>
    </input>

    <label id='exercise'>
        
        <?php
        echo randExer($var);
        ?>
    </label><br>
    <input type='number' id='solution' name='solution' placeholder="Solution" required/>
    <div style='display: inline-block; text-align: left;'>
        <input type="checkbox" id="consent" name="consent" value="consent" required="">
        <label>I agree with saving and sending this message according to the privacy policy.
        </label>
    </div>
    <input style='' type="submit" value="Send"/>
</form>

randomExercise.php:

<?php

    $encryptionPassword = "***";

    function randExer($rand) {

        //========================
        //Change for customization
        //========================

        //First of all:
        //Please change the $encryptionPassword above (16 chars)

        //Width of the created image
        $width = 200;

        //Height of the created image
        $height = 50;

        //RGB values for the text on the black image
        $textColorRed = 255;
        $textColorGreen = 255;
        $textColorBlue = 255;

        //RGB values of the random lines on the image
        $linesRed = 192;
        $linesGreen = 192;
        $linesBlue = 192;

        //Value between 1 and 5
        $fontSize = 5;  

        //Coordinates where the text starts
        $upperLeftCornerX = 18;
        $upperLeftCornerY = 18;

        //Text will be rotated by $angle-degrees
        $angle = 10;

        global $encryptionPassword;


        //=============================================
        //From here no changes needed for customization
        //=============================================

        $random = openssl_decrypt($rand,"AES-128-ECB", $encryptionPassword);
        
        //Creates a black picture
        $img = imagecreatetruecolor($width, $height);

        //uses RGB-values to create a useable color
        $textColor = imagecolorallocate($img, $textColorRed, $textColorGreen, $textColorBlue);
        $linesColor = imagecolorallocate($img, $linesRed, $linesGreen, $linesBlue);

        //Adds text
        imagestring($img, $fontSize, $upperLeftCornerX, $upperLeftCornerY, $random . " = ?", $textColor);

        //Adds random lines to the images
        for($i = 0; $i < 5; $i++) {
            imagesetthickness($img, rand(1, 3));
            $x1 = rand(0, $width / 2);
            $y1 = rand(0, $height / 2);
            $x2 = $x1 + rand(0, $width / 2);
            $y2 = $y1 + rand(0, $height / 2);
            imageline($img, $x1, $x2, $x2, $y2, $linesColor);
        }

        $rotate = imagerotate($img, $angle, 0);

        //Attribution: https://stackoverflow.com/a/22266437/13634030
        ob_start();
            imagejpeg($rotate);
            $contents = ob_get_contents();
        ob_end_clean();
        $imageData = base64_encode($contents);
        $src = 'data:'. mime_content_type($contents) . ';base64,' . $imageData;
        return '<img alt="" src="' . $src . '">';
    };

    function randText() {
        global $encryptionPassword;

        //Creating random (simple) math problem
        $arr = array("zero", "one", "two", "three", "four", "five", "six", "seven", "eight", "nine", "ten");
        $item1 = $arr[array_rand($arr)];
        $item2 = $arr[array_rand($arr)];
        $random = $item1 . " + " . $item2;
        $encrypted = openssl_encrypt($random,"AES-128-ECB", $encryptionPassword);
        return $encrypted;
    }
?>

send.php

<?php
    
    //Get simple math-problem (e.g. four + six)
    $str = openssl_decrypt($_REQUEST['exerciseText'], "AES-128-ECB", "***");
    
    $first = strpos($str, " ");

    //Get first number (e.g. four)
    $substr1 = substr($str, 0, $first);

    //Get second number (e.g. six)
    $substr2 = substr($str, $first + 3, strlen($str) - $first - 3);
    $arr = array("zero", "one", "two", "three", "four", "five", "six", "seven", "eight", "nine", "ten");
    /*
     * Convertring strings to numbers, e.g.
     * four -> 4
     * six  -> 6
     */

    $x = 0;
    $y = 0;

    for($i = 0; $i <= 10; $i++) {
        if(strcmp($substr1, $arr[$i]) == 0) {
            $x = $i;
            break;
        }
    }

    for($i = 0; $i <= 10; $i++) {
        if(strcmp($substr2, $arr[$i]) == 0) {
            $y = $i;
            break;
        }
    }

    $z = intval($_POST['solution']);

    //Did user enter right solution?
    if($z == ($x + $y)) {
        
        //Bot filled the honeypott-tree
        if(!empty($_POST['website'])) {
            header("Location:/sites/messageError.html");
            die();
        }

        $userName = $_POST['myName'];
        $userEmail = $_POST['myEmail'];
        $userMessage = $_POST['myMessage'];

        //Did user enter a valid email-adress?
        if(!filter_var($userEmail, FILTER_VALIDATE_EMAIL)) {
            header("Location:http:///sites/messageError.html");
            die();
        }

        //Creating message
        $to = "***";
        $subject = "New Contact-form message";
        $body = "Content:";

        $body .= "\n\n Name: " . $userName;
        $body .= "\n\n Email: " . $userEmail;
        $body .= "\n\n Message: " . $userMessage;

        //Trying to send message
        if(mail($to, $subject, $body)){
            header("/sites/message.html");
            die();
        } else{
            header("Location:/sites/messageError.html");
            die();
        }
    }

    header("Location:/sites/messageError.html");
?>

Questions

All suggestions are welcome, but I am especially interested in the security of this approach and how to further improve it.


Edit: I just created a git-repository and a working demo. Maybe that's helpful for review.

\$\endgroup\$
4
+50
\$\begingroup\$

Speaking about security, there are several issues:

  • As a rule the attacker will go the easy way: send and analyze first request, remember exerciseText and solution, then send an unlimited number of requests from an unlimited number of devices. So, at least, you should use a timestamp to shorten the token's lifetime and tie it to a specific IP.

  • In fact your encryption function makes no sense, because there are only 121 solutions and the attacker will notice very quickly that the solution always has the same value for same exerciseText. Therefore, you should at least append an UUID to your $random variable.

  • Although, in your case the attacker can bypass your CAPTACHA just by sending exerciseText=0&solution=0&.... It's because there are no input validations and as a result your math verification comes down to if(0 == (0 + 0)). Moreover, on improperly configured servers it may lead to Full Path Disclosure.

As a last word I would like to say that it is very difficult to develop a spam free form without storing tokens into sessions or database. Well, your solution may be useful for learning and low traffic websites, but it's useless for popular ones.

UPDATE

For a full PoC that bypasses CAPTCHA, check this cURL example:

curl -v https://demo.philippwilhelm.eu/send.php -d "myName=a&myEmail=a@b.c&myMessage=m&exerciseText=0&solution=0&consent=consent"

As a result your server returns following headers:

< HTTP/1.1 302 Found
< Date: Thu, 10 Sep 2020 16:17:50 GMT
* Server Apache is not blacklisted
< Server: Apache
< Upgrade: h2
< Connection: Upgrade
< Location: https://philippwilhelm.eu/sites/message.html
< Content-Length: 0
< Content-Type: text/html; charset=utf-8

Considering that server redirects the request to /message.html it means that message was successfully sent. So, let see what's wrong with your code (I removed your comments and added clarifications for each critical point):

# If attacker submits an empty/invalid `exerciseText` the `openssl_decrypt()` is not able to decrypt message and returns `FALSE` to `$str`
$str = openssl_decrypt($_REQUEST['exerciseText'], "AES-128-ECB", "***");

$first = strpos($str, " ");

# Both `$substr1` and `$substr2` will be assigned `FALSE` because both `substr()` are trying to extract a portion of string from a `FALSE` value
$substr1 = substr($str, 0, $first);
$substr2 = substr($str, $first + 3, strlen($str) - $first - 3);

$arr = array("zero", "one", "two", "three", "four", "five", "six", "seven", "eight", "nine", "ten");

# Both `$x` and `$y` will remain equal to zero because in both cases `strcmp()` is not able to "convert strings to numbers"
$x = 0;
$y = 0;

for($i = 0; $i <= 10; $i++) {
    if(strcmp($substr1, $arr[$i]) == 0) {
        # The `$substr1` is `FALSE`, so `$i` is never assigned to `$x`
        $x = $i;
        break;
    }
}

for($i = 0; $i <= 10; $i++) {
    if(strcmp($substr2, $arr[$i]) == 0) {
        # The `$substr2` is `FALSE`, so `$i` is never assigned to `$y`
        $y = $i;
        break;
    }
}

# If attacker submits an empty `solution` the `$z` will be assigned the value `0` (zero)
$z = intval($_POST['solution']);

# Finally `$z=0; $x=0; $y=0` so the following expression will be always true
if($z == ($x + $y)) {
    ...
}

To avoid such vulnerabilities you must validate user input before before using it. So, let's rewrite the lines above to make sure that attacker won't be able to cheat on your validation:

# PHP has built-in functions for obtaining, validating and sanitizing submitted data, therefore in most cases you don't need to use superglobals
$solution = filter_input(INPUT_POST, 'solution', FILTER_VALIDATE_INT);
$exerciseText = filter_input(INPUT_POST, 'exerciseText');

# Since in both cases `filter_input()` returns `FALSE` if something is wrong, you should not go further if any of these variables are invalid
if ($solution === false || $exerciseText === false) {
    die('bad request');
}

# Next you must make sure that `$exerciseText` has been successfully decrypted, otherwise exit your script
$str = openssl_decrypt($exerciseText, "AES-128-ECB", "***");
if (!$str) {
    die('cannot decrypt exercise');
}

# Define your numbers array
$arr = array("zero", "one", "two", "three", "four", "five", "six", "seven", "eight", "nine", "ten");

# Since the `$str` contains several numbers separated by `+`, just split them and strip spaces
$words = array_map('trim', explode('+', $str)); //-> ["one", "two"]

# As the numbers match the indexes of `$arr`, you can find them using `array_intersect()`
$numbers = array_intersect($arr, $words); //-> [1=>"one", 2=>"two"]

# Make sure that it found two numbers
if (count($numbers) != 2) {
    die('hacking attempt');
}

# Sum up the indexes for the found numbers
$sum = array_sum(array_keys($numbers));

# Finally, check if user submitted a correct solution
if ($solution == $sum) {
    ...
}
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ codereview.stackexchange.com/questions/248695/… \$\endgroup\$ – mickmackusa Sep 10 at 10:49
  • \$\begingroup\$ I don't really understand the third point. Could you explain where (and how) I should validate the input? \$\endgroup\$ – Philipp Wilhelm Sep 10 at 12:24
  • 1
    \$\begingroup\$ @PhilippWilhelm I updated my answer and added more details. \$\endgroup\$ – Victor Sep 10 at 18:08
1
\$\begingroup\$

Security

The code basically attempts to create a Client-side Request Forgery (CSRF) token. Some frameworks like laravel provide this functionality by default. It may not be worth the effort of incorporating a framework if all you need is one feature but if there were others then you might consider it.

Generation of a CSRF token would be much simpler than creating an image an encoding it into string format.

As this StackOverflow answer explains a CSRF token can be generated with various techniques. Hopefully the server running the code is using PHP 7.2 or later given Supported versions. Presuming that is the case random_bytes() to generate a cryptographically secure string of pseudo-random bytes and bin2hex() can be used to convert that string to a hexadecimal representation. As Victor's answer mentions a session can be used to store the token. A session can be started with session_start()

session_start();
$_SERVER['csrf_token'] = bin2hex(random_bytes(32));

Then that token can be included in the form:

<form method="POST" action="../php/send.php" class='input'>
    <input type="hidden" name="csrf_token" value="<?=$_SERVER['csrf_token']?>" />

Then when the form is submitted, that value can be compared with the session value:

<?php
if ($_POST['csrf_token'] !== $_SERVER['csrf_token']) {
    header("Location:/sites/messageError.html");
}

This is especially important if PHP code should be executed before HTML or other output is emitted - e.g. sending headers.

Review remarks

PHP within HTML

Instead of this:

<input id='exerciseText' name='exerciseText', style='display: none;' value='
   <?php
   include '../php/randomExercise.php';
   $var = randText();
   echo $var;
   ?>'>

The external PHP file can be included at the top of the file:

<?php
include '../php/randomExercise.php';
?>
 <html> <!-- continue HTML below -->

Then in the form the function can be called using the shortcut syntax for echo - i.e. <?= ?>:

 <input id='exerciseText' name='exerciseText', style='display: none;' value='<?=randText()?>'>

HTML

inline styles

Many elements have inline styles - e.g.

<textarea rows="8" name="myMessage" style='width: 100%; resize: none; border: 1px solid Gray; border-radius: 4px; box-sizing: border-box; padding: 10px 10px;' placeholder="Message" required></textarea><br><br>

Those styles could be moved to a <style> tag or external stylesheet so the markup won't contain so much styling.

p.s. padding: 10px 10px; can be condensed to padding: 10px;

email input type

There is an input for email address:

<input type="text" name="myEmail" placeholder="E-Mail" required/><br><br>

The type attribute could be changed to email for validation purposes.

PHP

Avoid else keyword when not necessary

In this presentation about cleaning up code Rafael Dohms talks about many ways to keep code lean - like avoiding the else keyword. (see the slides here).

It is wise to avoid the else keyword - especially when it isn't needed - e.g. when a previous block contains a return statement or call to die() - for example:

   if(mail($to, $subject, $body)){
       header("/sites/message.html");
       die();
   } else{
       header("Location:/sites/messageError.html");
       die();
   }

p.s. should there be a Location: here?

 header("/sites/message.html");

store encryption password in constant instead of global variable

In general usage of global variables is frowned upon for many reasons -e.g. because program state can be unpredictable, testing can be more difficult, etc. (refer to answers to Why is Global State so Evil? for more information). A constant can be declared with declare() or the const() keyword.

splitting strings

In send.php sub-string functions are used to split the string into words. The PHP function explode() could be used with list() to assign to $substr1 and $substr2 in a single line.

generating number names

The method NumberFormatter::format() class and the range() function could be used to generate the number names in $arr.

$fmt = new NumberFormatter( 'en', NumberFormatter::SPELLOUT);
$arr = [];
foreach(range(0,10) as $number) {
    $arr[] = $fmt->format($number);
}

Bearing in mind that a beginner might be unfamiliar with such advanced techniques, the array could be generated with array_map():

$fmt = new NumberFormatter( 'en', NumberFormatter::SPELLOUT );
$arr = array_map($fmt->format, range(0, 10));

Searching for numbers by name

The PHP function array_search() could likely be used to eliminate the for loops to assign values for $x and $y.

| improve this answer | |
\$\endgroup\$

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.