PHP form that posts to Trello API and mailgun API

Question

The following is the full code of a page that receives a POST request from another page (I don't believe it's too relevant what that other page is, but I'll post it if requested).

Assuming that a request to this page can be made with arbitrary POST data, are there any glaring vulnerabilities?

I'm mainly concerned about the strings being passed to the page via the POST data, but other vulnerabilities would be good as well.

This is being run on an nginx server, the user input is validated client-side but not server-side, there are no db actions performed by the php here.

<!DOCTYPE HTML>
<html>
    <head>
        <title>Your request has been submitted</title>
        <?php include("html-head.php") ?>
    </head>
    <body class="index">
            <?php include_once("header.php") ?>

        <?php

        if (!isset($_POST['username'])){
            die();
        } else {
            $username = trim(str_replace("@","",$_POST['username']));
        }

        $hotlist = array('harpies');
        function get_ip_address() {
        if (!empty($_SERVER['HTTP_CLIENT_IP']) && $this->validate_ip($_SERVER['HTTP_CLIENT_IP']))
        return $_SERVER['HTTP_CLIENT_IP'];
        if (!empty($_SERVER['HTTP_X_FORWARDED_FOR'])) {
        $iplist = explode(',', $_SERVER['HTTP_X_FORWARDED_FOR']);
        foreach ($iplist as $ip) {
         if ($this->validate_ip($ip))
          return $ip;
        }
        $logstr = time();
        foreach ($_POST as $key => $value) {
            $log .="\t".$key."\t".$value."\r\n";
        }
        }
        if (!empty($_SERVER['HTTP_X_FORWARDED']) && $this->validate_ip($_SERVER['HTTP_X_FORWARDED']))
        return $_SERVER['HTTP_X_FORWARDED'];
        if (!empty($_SERVER['HTTP_X_CLUSTER_CLIENT_IP']) && $this->validate_ip($_SERVER['HTTP_X_CLUSTER_CLIENT_IP']))
        return $_SERVER['HTTP_X_CLUSTER_CLIENT_IP'];
        if (!empty($_SERVER['HTTP_FORWARDED_FOR']) && $this->validate_ip($_SERVER['HTTP_FORWARDED_FOR']))
        return $_SERVER['HTTP_FORWARDED_FOR'];
        if (!empty($_SERVER['HTTP_FORWARDED']) && $this->validate_ip($_SERVER['HTTP_FORWARDED']))
        return $_SERVER['HTTP_FORWARDED'];
        return $_SERVER['REMOTE_ADDR'];
        }
        $currentURL = explode("/",$_SERVER['REQUEST_URI'])[0];
        $errmsg = "";
         $trustedUsers = array("user1","user2");
        $userIDs = array(
            "user1"=>"xxxxxxxxxxxxxxxxxxxxxxxx",
            "user2"=>"xxxxxxxxxxxxxxxxxxxxxxxx",
            "user3"=>"xxxxxxxxxxxxxxxxxxxxxxxx"
            );
        $userName = $trustedUsers[array_rand($trustedUsers)];
        function rand_color() {
            return str_pad(dechex(mt_rand(0, 0xFFF)), 3, '0', STR_PAD_LEFT);
        }
        $rndCode = strtoupper(rand_color());

        $logstr = time('Y-m-d H:i:s');
        foreach ($_POST as $key => $value) {
            $logstr .= "\t".$key."\t".$value."\r\n";
        }
        $logstr .= "\tipAddr\t".get_ip_address()."\r\n";
        $logstr .= "\trndCode\t".$rndCode."\r\n";
        $logstr .= "\ttrustedUsers\t".$userName."\r\n";
        $logUrl = "http://example.com/log.php";
        $logFields = "str=".$logstr;
        $cl = curl_init();
        curl_setopt($cl, CURLOPT_URL, $logUrl);
        curl_setopt($cl, CURLOPT_POST, 1);
        curl_setopt($cl, CURLOPT_POSTFIELDS, $logFields);
        $result = curl_exec($cl);
        curl_close($cl);

        // require_once "Mail.php";
        $emailFieldstrings = "";
        $emailURL = "https://api.mailgun.net/v2/example.com/messages";
        $email_Fields = array(
                "from" => "no reply <[email protected]>",
                "to" => $username." <".$_POST['email'].">",
                "bcc" => "[email protected]",
                "subject" => "Action needed to complete your request",
                "text" => "Email content.",
                "html" => "<html><body>Email content </body> </html>"
                );
        $emailUser = "api:key-d7ca70c3e32e82108928322ea23bbee8";

        foreach($email_Fields as $key=>$value) { $emailFieldstrings .= $key.'='.$value.'&'; }
        rtrim($emailFieldstrings, '&');

        $cu = curl_init();

        curl_setopt($cu,CURLOPT_URL, $emailURL);
        curl_setopt($cu,CURLOPT_USERPWD, "api:key-d7ca70c3e32e82108928322ea23bbee8");
        curl_setopt($cu,CURLOPT_POST, count($email_Fields));
        curl_setopt($cu,CURLOPT_POSTFIELDS, $emailFieldstrings);

        $result = curl_exec($cu);

        curl_close($cu);
        $bit = array(
          "ign" => $username,
          "email" => $_POST['email'],
          "ip" => get_ip_address()
);

$matchlist = array();

$hotlist = array(
  "purple" => "purple category",
  "red" => "red category",
  "orange" => "orange category"
);

$data = "115.186.242.130";

foreach($bit as $desc => $data){
foreach($hotlist as $key => &$value){
  $ch = curl_init();
  curl_setopt($ch, CURLOPT_URL, "https://api.trello.com/1/search?token=xxxx&key=xxxxxx&card_fields=name,url,desc&partial=true&query=label:" . $key . "%20"  . urlencode($data));
  curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1);
  $output = curl_exec($ch);
  curl_close;
  $output = json_decode($output, true);
  if ($output["cards"]){
    foreach ($output["cards"] as $card){
      $id = $card['id'];
      $matchlist[$id]="[" . $card['name'] . "](" . $card['url'] . ")";
    }
 }
}}

$dup_warn = "";

foreach ($matchlist as $key => &$value){
  $dup_warn = "\n- " . $value;
}

if (count($dup_warn) > 0){
  $dup_warn = "Some information from this application matched information from the following other applications:" . $dup_warn;
}

        if (strlen($errmsg) > 0){
            $details = $dup_warn . "Username Provided: ".$username."\nAreas of play: ".$_POST['areas']."\n Email: ".$_POST['email']."\nG+ Profile: ".$_POST['gplus']."\nIP Address: ".get_ip_address().$errmsg;

            $url = "https://api.pushover.net/1/messages.json";
            $fields = array(
                    'token' => urlencode("zzzzzzzzzzzzzzzzzzzzzzzz"),
                    'user' => urlencode("xzzxzxzzzzzzzzzzzzzzzz"),
                    'title' => urlencode("Something went wrong with your site"),
                    'message' => urlencode($details),
                );
            foreach($fields as $key=>$value) { $fields_string .= $key.'='.$value.'&'; }
            rtrim($fields_string, '&');
            $ch = curl_init();
        }
        $trelloCard = $dup_warn;

        if (in_array($username, $hotlist)){
            $trelloCard .= "--- \xA ** Warning: User on hotlist, consult mods before adding.**";
        }

        if (isset($_POST['areas'])){
            $trelloCard .= "\xA 1. **Areas**: ".$_POST['areas']." ";
        }

        $trelloCard .= "\xA 2. **Email**: ".$_POST['email']." \xA 3. **Google+ Profile**: [link](".$_POST['gplus'] . ") ";

        if(strlen(trim($_POST['referrer']))){
            $trelloCard .= "\xA 4. **The following user(s) have been listed as references: " . trim($_POST['referrer'])."**";
        }else{
            $trelloCard .= "\xA 5. **This application did not list any references.**";
        }

        $trelloCard .= "\xA \xA ------- \xA \xA 6. Handler: ".$userName."\xA 7.  _IP Address: ".get_ip_address()."_ \xA 8. Slack: [invite](https://slack.com/api/users.admin.invite?email=" . $_POST['email'] . "&token=xxxxxxxxxxxx&set_active=true&_attempts=1) \xA 9. Browser: ".$_POST['useragent'];


        $url = 'https://api.trello.com/1/cards';
        $fields = array(
            'key' => urlencode('xxxx'),
            'token' => urlencode('xxxxxxxx'),
            'name' => urlencode($rndCode." - ".$username),
            'due' => date('Y.m.d', strtotime('+1 Week')),
            'desc' => urlencode($trelloCard),
            'pos' => urlencode('top'),
            'idList' => urlencode('xxxxxxxxxxxxxxx'),
            'idMembers' => urlencode($userIDs[$userName])
            // 'idMembers' => urlencode($userIDs['jimsug'])
            );
        $fields_string='';
        foreach($fields as $key=>$value) { $fields_string .= $key.'='.$value.'&'; }
        $fields_string=rtrim($fields_string, '&');
        $ch = curl_init();
        curl_setopt($ch,CURLOPT_RETURNTRANSFER, 1);
        curl_setopt($ch,CURLOPT_URL, $url);
        curl_setopt($ch,CURLOPT_POST, count($fields));
        curl_setopt($ch,CURLOPT_POSTFIELDS, $fields_string);
        $result = curl_exec($ch);
        curl_close($ch);
        ?>
        <section id="banner">
                <div class="inner">
                    <header>
                        <h2>Thanks</h2>
                    </header>
                    <p>We've received your request </br>
                    We'll be in touch
                    </p>
                </div>
                <header class="special container">
                <p>You will receive an email from us with a <strong>verification</strong> code to send to a <strong>trusted user</strong>.</p>
                </header>
            </section>
<!--         <article id="main">
        </article> -->
<?php include_once("footer.php") ?>
    </body>
</html>

Mike Brant · Accepted Answer · 2017-06-13 15:53:57Z

You need to get into the habit of performing all of your business logic before your start outputting to the browser. Say someone posted bad data and you wanted to redirect to a 403 page using header(). You can't do that now, because you have already started output. Right now your code could also lead to malformed HTML. What happens if $_POST is not set? You get an open body tag. Similarly, you have some HTMl output near the end of the script which seem to be output without regard to success of intermediate API calls.

You need to perform all your data validations, API interactions, etc. up front in order to determine application state that should be presented to user and then present that state. This can greatly eliminate a lot of your nested conditionals and such when it comes time to render output.

Don't have functions like get_ip_address() and rand_color() tucked away in the middle of code that is creating side effects, like outputting to user, changing data stores, etc. If you are not familiar with the PHP Standards Recommendations, I would highly recommend that you familiirize yourself with them and try to follow them. This is pretty much the best set of standards that exists for trying to codify what the behaviors for professional PHP development should be. Note that in PSR-1 (basic coding standards), one of the first sections talks about side effects, stating:

A file SHOULD declare new symbols (classes, functions, constants, etc.) and cause no other side effects, or it SHOULD execute logic with side effects, but SHOULD NOT do both.

If you were following this standard, your function definitions would be in a totally different file.

Why are you using $this inside of your get_ip_address() function? There is no object context here at all, and my guess is your code doesn't actually work right now.

Consider filter_input() or filter_input_array() for use in validating/sanitizing your input data. You should absolutely not be working with POST data (especially sending it off to other servers), until you have validated/sanitized it. Here you just work with it freely, just hoping for the best. This is a no security posture. So when you ask about application security, please understand that you effectively have no security right now.

Indent your code properly. Your code is really hard to read. Everything inside curly braces, should be indented one level. Even if you have control structures like conditionals without braces (which would be non-compliant with PSR-2 style guide), you should indent the following line so it is clear that the line is within the control structure.

Example:

if (!empty($_SERVER['HTTP_CLIENT_IP']) && $this->validate_ip($_SERVER['HTTP_CLIENT_IP']))
    return $_SERVER['HTTP_CLIENT_IP'];

You also have case where it looks like you are "indenting" with single spaces, with really isn't indentation at all.

Why are you storing authorization info (your trusted users array) in this file?

Why should this be hard-coded here? Do you really want to have to change and redeploy this file just to make an authorization change?

Why are your randomly choosing a "trusted user" for use in this script? This logic seems odd.

You do not seem to be doing any mitigation against cross-site request forgery (CSRF) attacks, meaning anyone can hijack this endpoint and being sending out mail.

Ideally, you should keep your passwords, API keys, etc. out of your code and inject via configuration. Why do you hard-code your API key twice here?

Billal BEGUERADJ · Accepted Answer · 2017-06-13 11:45:39Z

Post data validation: First of all, you need to more work on data validation. For instance, you accept almost everything as a valid $username.
Data sanitation: Even if you can not figure out right away how a nefarious attacker could hack your web application, you must sanitize your $_POST data on a larger scale. For this purpose, I suggest you to design a generic function through which all $_POST data must go:
```
function sanitize_input($data) {
    /* Run different sanitization functions here:
       - trim();
       - stripslashes()
       - htmlspecialchars()
       - ...
    */ 
    return $data;
}
```
I feel lazy to go through all the instructions of your code, however you seem to call curl_exec() 5 times. There are known security issues with this function, thus you need to filter your data more properly.

I wrestled a bear once. · Accepted Answer · 2017-06-22 20:30:07Z

You're doing several things with curl. Every time you make a request with curl you are adding ~10 lines to your code. Create a curl function like this one instead. That alone will make your code considerably more readable.
Do your logic before the <DOCTYPE> Once you start outputting HTML, PHP shouldn't be doing anything but echoing parts of the HTML.
This line is lonely and out of place: $data = "115.186.242.130"; Send him home. This one too: $logstr = time(); ...you have a lot of code that doesn't actually do anything. You're initiating curl at least once and not actually making a request.

On that note, this code is nowhere near ready for review...

Stack Exchange Network

PHP form that posts to Trello API and mailgun API

3 Answers 3

You must log in to answer this question.

Hot Network Questions

PHP form that posts to Trello API and mailgun API

3 Answers 3

You must log in to answer this question.

Related

Hot Network Questions