Can my simple php contact form be hijacked for anything mal

Discussions of secure PHP coding. Security in software is important, so don't be afraid to ask. And when answering: be anal. Nitpick. No security vulnerability is too small.

Moderator: General Moderators

Post Reply
jarednz
Forum Newbie
Posts: 1
Joined: Thu Apr 08, 2010 8:56 pm

Can my simple php contact form be hijacked for anything mal

Post by jarednz »

Hi all

I was wondering if someone could take a quick peak at my code and let me know if I have any major security flaws in my code. Such as any variables that could be hijacked for any injection or methods that could be used to get access to our web server, that sort of stuff.

Its a simple contact form built in php, takes values from fields in a form and posts it to an email address. There is no database back end and while the data is "not" top secret (being a contact form n all) still be nice to know if it can't be hijacked and forwarded off to a spam service or something crazy :)

Code: Select all

<?php

function validEmail($email)
{
   $isValid = true;
   $atIndex = strrpos($email, "@");
   if (is_bool($atIndex) && !$atIndex)
   {
      $isValid = false;
   } else {
      $domain = substr($email, $atIndex+1);
      $local = substr($email, 0, $atIndex);
      $localLen = strlen($local);
      $domainLen = strlen($domain);
      
      if ($localLen < 1 || $localLen > 64)
      {
         // local part length exceeded
         $isValid = false;
      }
      else if ($domainLen < 1 || $domainLen > 255)
      {
         // domain part length exceeded
         $isValid = false;
      }
      else if ($local[0] == '.' || $local[$localLen-1] == '.')
      {
         // local part starts or ends with '.'
         $isValid = false;
      }
      else if (preg_match('/\\.\\./', $local))
      {
         // local part has two consecutive dots
         $isValid = false;
      }
      else if (!preg_match('/^[A-Za-z0-9\\-\\.]+$/', $domain))
      {
         // character not valid in domain part
         $isValid = false;
      }
      else if (preg_match('/\\.\\./', $domain))
      {
         // domain part has two consecutive dots
         $isValid = false;
      }
      else if
      (!preg_match('/^(\\\\.|[A-Za-z0-9!#%&`_=\\/$\'*+?^{}|~.-])+$/', str_replace("\\\\","",$local)))
      {
         // character not valid in local part unless 
         // local part is quoted
         if (!preg_match('/^"(\\\\"|[^"])+"$/', str_replace("\\\\","",$local)))
         {
            $isValid = false;
         }
      }
      if ($isValid && !(myCheckDNSRR($domain,"MX") || myCheckDNSRR($domain,"A")))
      {  // domain not found in DNS
         $isValid = false;
      }
   }
   return $isValid;
}

function myCheckDNSRR($hostName, $recType = '')
{
 if(!empty($hostName))
 {
   if( $recType == '' ) $recType = "MX";
   exec("nslookup -type=$recType $hostName", $result);
   // check each line to find the one that starts with the host
   // name. If it exists then the function succeeded.
   foreach ($result as $line)
   {
     if(eregi("^$hostName",$line))
     {
       return true;
     }
   }
   // otherwise there was no mail handler for the domain
    return false;
 }
  return false;
}




$name = trim($_REQUEST['name']); 
$emailCheck = trim($_REQUEST['email']);
$phone = trim($_REQUEST['phone']);
$EnquirySubject = $_REQUEST['EnquirySubject'];
$queryComments = trim($_REQUEST['queryComments']);

         switch ($_REQUEST['EnquirySubject'])
         {
          case "General land information":
            $checkedSubject0 = 'checked="checked"';
            $checkedSubject1 = "";
            $checkedSubject2 = "";
            $checkedSubject3 = "";
            $checkedSubject4 = "";
            $checkedSubject5 = "";
            $checkedSubject6 = "";
            $checkedSubject7 = "";
            $checkedSubject8 = "";
            $checkedSubject9 = "";
            $checkedSubject10 = "";
            $checkedSubject11 = "";
          break;
          	
          case "How to order a land record, eg. Title":
            $checkedSubject0 = ""; 
            $checkedSubject1 = 'checked="checked"';
            $checkedSubject2 = "";
            $checkedSubject3 = "";
            $checkedSubject4 = "";
            $checkedSubject5 = "";
            $checkedSubject6 = "";
            $checkedSubject7 = "";
            $checkedSubject8 = "";
            $checkedSubject9 = "";
            $checkedSubject10 = "";
            $checkedSubject11 = "";
          break;
          
          case "Geodetic mark updates and information":
            $checkedSubject0 = ""; 
            $checkedSubject1 = "";
            $checkedSubject2 = 'checked="checked"';
            $checkedSubject3 = "";
            $checkedSubject4 = "";
            $checkedSubject5 = "";
            $checkedSubject6 = "";
            $checkedSubject7 = "";
            $checkedSubject8 = "";
            $checkedSubject9 = "";
            $checkedSubject10 = "";
            $checkedSubject11 = "";
          break;
          
          case "online":
            $checkedSubject0 = ""; 
            $checkedSubject1 = "";
            $checkedSubject2 = "";
            $checkedSubject3 = 'checked="checked"';
            $checkedSubject4 = "";
            $checkedSubject5 = "";
            $checkedSubject6 = "";
            $checkedSubject7 = "";
            $checkedSubject8 = "";
            $checkedSubject9 = "";
            $checkedSubject10 = "";
            $checkedSubject11 = "";
          break;
          
          case "Maps":
            $checkedSubject0 = "";
            $checkedSubject1 = "";
            $checkedSubject2 = "";
            $checkedSubject3 = "";
            $checkedSubject4 = 'checked="checked"';
            $checkedSubject5 = "";
            $checkedSubject6 = "";
            $checkedSubject7 = "";
            $checkedSubject8 = "";
            $checkedSubject9 = "";
            $checkedSubject10 = "";
            $checkedSubject11 = "";
          break;
          
          case "Hydrographic information":
            $checkedSubject0 = ""; 
            $checkedSubject1 = "";
            $checkedSubject2 = "";
            $checkedSubject3 = "";
            $checkedSubject4 = "";
            $checkedSubject5 = 'checked="checked"';
            $checkedSubject6 = "";
            $checkedSubject7 = "";
            $checkedSubject8 = "";
            $checkedSubject9 = "";
            $checkedSubject10 = "";
            $checkedSubject11 = "";
          break;
          
          case "Our website":
            $checkedSubject0 = ""; 
            $checkedSubject1 = "";
            $checkedSubject2 = "";
            $checkedSubject3 = "";
            $checkedSubject4 = "";
            $checkedSubject5 = "";
            $checkedSubject6 = 'checked="checked"';
            $checkedSubject7 = "";
            $checkedSubject8 = "";
            $checkedSubject9 = "";
            $checkedSubject10 = "";
            $checkedSubject11 = "";
          break;
          
          case "OIA Requests":
            $checkedSubject0 = ""; 
            $checkedSubject1 = "";
            $checkedSubject2 = "";
            $checkedSubject3 = "";
            $checkedSubject4 = "";
            $checkedSubject5 = "";
            $checkedSubject6 = ""; 
            $checkedSubject7 = 'checked="checked"';
            $checkedSubject8 = "";
            $checkedSubject9 = "";
            $checkedSubject10 = "";
            $checkedSubject11 = "";
          break;
 
          case "Survey Mark Protection Service":
            $checkedSubject0 = ""; 
            $checkedSubject1 = "";
            $checkedSubject2 = "";
            $checkedSubject3 = "";
            $checkedSubject4 = "";
            $checkedSubject5 = "";
            $checkedSubject6 = ""; 
            $checkedSubject7 = "";
            $checkedSubject8 = 'checked="checked"';
            $checkedSubject9 = "";
            $checkedSubject10 = "";
            $checkedSubject11 = "";
          break;
          case "Report damage or disturbance to survey marks":
            $checkedSubject0 = ""; 
            $checkedSubject1 = "";
            $checkedSubject2 = "";
            $checkedSubject3 = "";
            $checkedSubject4 = "";
            $checkedSubject5 = "";
            $checkedSubject6 = ""; 
            $checkedSubject7 = "";
            $checkedSubject8 = ""; 
            $checkedSubject9 = 'checked="checked"';
            $checkedSubject10 = "";
            $checkedSubject11 = "";
          break;
          case "Recommendations for additional survey control":
            $checkedSubject0 = ""; 
            $checkedSubject1 = "";
            $checkedSubject2 = "";
            $checkedSubject3 = "";
            $checkedSubject4 = "";
            $checkedSubject5 = "";
            $checkedSubject6 = ""; 
            $checkedSubject7 = "";
            $checkedSubject8 = "";
            $checkedSubject9 = "";
            $checkedSubject10 = 'checked="checked"';
            $checkedSubject11 = "";
          break;
          case "Other":
            $checkedSubject0 = ""; 
            $checkedSubject1 = "";
            $checkedSubject2 = "";
            $checkedSubject3 = "";
            $checkedSubject4 = "";
            $checkedSubject5 = "";
            $checkedSubject6 = "";
            $checkedSubject7 = ""; 
            $checkedSubject8 = "";
            $checkedSubject9 = "";
            $checkedSubject10 = "";
            $checkedSubject11 = 'checked="checked"';
          break;
         }


function displayForm($name, $email, $phone, $EnquirySubject, $queryComments, $checkedSubject0, $checkedSubject1, $checkedSubject2, $checkedSubject3, $checkedSubject4, $checkedSubject5, $checkedSubject6, $checkedSubject7, $checkedSubject8, $checkedSubject9, $checkedSubject10, $checkedSubject11, $phoneError)
{
  //make $emailCheck global so function can get value from global scope.
  global $emailCheck;
  
  
  //name
  echo  '<form action="index.php" method="post" name="contact" id="contact">'."\n".
        '<fieldset>'."\n".
        '<div>'."\n".
        '<label for="name">Your name:</label>'."\n".
        '<input type="text" name="name" id="name" class="inputText required" value="'. $name .'" />'."\n";
        
        //check if name field is filled out
        if (isset($_REQUEST['submit']) && empty($name)) 
        {        
          echo '<label for="name" class="error">Please enter your name.</label>'."\n";
        }
        
   echo '</div>'."\n". '<div>'."\n";
   
   //Email     
   echo '<label for="email">Your email:</label>'."\n".
        '<input type="text" name="email" id="email" class="inputText required email" value="'. $emailCheck .'" />'."\n";
      
       // check if email field is filled out and proper format   
        if (isset($_REQUEST['submit']) && validEmail($emailCheck) == false)
        {
          echo '<label for="email" class="error">Invalid email address entered.</label>'."\n";
        }
        
   echo '</div>'."\n". '<div>'."\n";
        
  //phone     
   echo '<label for="phone">Your phone number:</label>'."\n".
        '<input type="text" name="phone" id="phone" class="inputText" value="'. $phone .'" />'."\n".
		'<span class="mandatory small">(optional)</span>';
       
        
       // check if phone field is filled out that it has numbers and not characters

        if (isset($_REQUEST['submit']) && $phoneError == "true")
        {			
	        echo '<label for="email" class="error">Please enter a valid phone number.</label>'."\n";
        }
         
        
   echo '</div>'."\n". '</fieldset>'."\n".'<fieldset>'. "\n" . '<div>'."\n";
    
    //subect of enquiry
    echo 
         '<p style="padding-left: 1em">Subject of your enquiry:</p>';
         
         // check if email field is filled out and proper format   
         if (isset($_REQUEST['submit']) && empty($EnquirySubject)) 
         {
          echo '<label class="error" style="float: none !important;clear:both">These fields are required.</label><br />'."\n";
         }         
              
    echo '<div class="radioError"></div>';
    echo  '<p><label class="contactRadio" for="Subject_0"><input type="radio" name="EnquirySubject" value="General land information" id="Subject_0" '. $checkedSubject0 .'  /> General land information</label>'."\n\r".
          '<br />'."\n\r".
          '<label class="contactRadio" for="Subject_1"><input type="radio" name="EnquirySubject" value="How to order a land record, eg. Title" id="Subject_1" '. $checkedSubject1 .' /> How to order a land record, eg. Title</label>'."\n\r".
          '<br />'."\n\r".
          '<label class="contactRadio" for="Subject_2"><input type="radio" name="EnquirySubject" value="Geodetic mark updates and information" id="Subject_2" '. $checkedSubject2 .' /> Geodetic mark updates and information</label>'."\n\r".
          '<br />'."\n\r".
          '<label class="contactRadio" for="Subject_3"><input type="radio" name="EnquirySubject" value="online" id="Subject_3" '. $checkedSubject3 .' /> online</label>'."\n\r".
          '<br />'."\n\r".
          '<label class="contactRadio" for="Subject_4"><input type="radio" name="EnquirySubject" value="Maps" id="Subject_4" '. $checkedSubject4 .' /> Maps</label>'."\n\r".
          '<br />'."\n\r".
          '<label class="contactRadio" for="Subject_5"><input type="radio" name="EnquirySubject" value="Hydrographic information" id="Subject_5" '. $checkedSubject5 .' /> Hydrographic information</label>'."\n\r".
          '<br />'."\n\r".
          '<label class="contactRadio" for="Subject_6"><input type="radio" name="EnquirySubject" value="Our website" id="Subject_6" '. $checkedSubject6 .' /> Our website</label>'."\n\r".
          '<br />'."\n\r".
          '<label class="contactRadio" for="Subject_7"><input type="radio" name="EnquirySubject" value="OIA Requests" id="Subject_7" '. $checkedSubject7 .' /> OIA Requests</label>'."\n\r".
          '<br />'."\n\r".
          '<label class="contactRadio" for="Subject_8"><input type="radio" name="EnquirySubject" value="Survey Mark Protection Service" id="Subject_8" '. $checkedSubject8 .' /> Survey Mark Protection Service</label>'."\n\r".
          '<br />'."\n\r".
          '<label class="contactRadio" for="Subject_9"><input type="radio" name="EnquirySubject" value="Report damage or disturbance to survey marks" id="Subject_9" '. $checkedSubject9 .' /> Report damage or disturbance to survey marks</label>'."\n\r".
          '<br />'."\n\r".
          '<label class="contactRadio" for="Subject_10"><input type="radio" name="EnquirySubject" value="Recommendations for additional survey control" id="Subject_10" '. $checkedSubject10 .' /> Recommendations for additional survey control</label>'."\n\r".
          '<br />'."\n\r".
          '<label class="contactRadio" for="Subject_11"><input type="radio" name="EnquirySubject" value="Other" id="Subject_11" '. $checkedSubject11 .' /> Other</label>'."\n\r".
          '<br /></p>';
          
    echo '</div>'."\n". '<div>'."\n";
        
   //comment/query     
   echo '<label class="queryComments" for="queryComments">Query/Comments:</label>'."\n".
        '<textarea name="queryComments" id="queryComments" class="required">'. $queryComments .'</textarea>'."\n";
        
        //check if message field is filled out
        if (isset($_REQUEST['submit']) && empty($_REQUEST['queryComments']))
        {
          echo '<label for="queryComments" class="error">This field is required.</label>'."\n";
        }
        
   echo '</div>'."\n". '</fieldset>';
   

      echo '<div class="submit"><input type="submit" name="submit" value="Submit" id="submit"  /></div>'.
           '<div class="clear"><p><br /></p></div>'.
           '<p class="contact-form">If you have a problem using this form please email us at <a href="mailto:blah@blahblahblahhblah.com">blah@blahblahblahhblah.com</a></p>'.
           '</form>'."\n";
}
if (isset($_REQUEST['submit']) && !empty($_REQUEST['phone']) && !is_numeric($_REQUEST['phone']))
{
	$phoneError = "true";
}

if(empty($name) || empty($emailCheck) || empty($EnquirySubject) || empty($queryComments) || validEmail($emailCheck) == false || $phoneError == "true")
{
    displayForm($name, $email, $phone, $EnquirySubject, $queryComments, $checkedSubject0, $checkedSubject1, $checkedSubject2, $checkedSubject3, $checkedSubject4, $checkedSubject5, $checkedSubject6, $checkedSubject7, $checkedSubject8, $checkedSubject9, $checkedSubject10, $checkedSubject11, $phoneError);
  
} else {
    
  //send email
  $to = "blah@blahblahblahhblah.com";
  $subject = "$EnquirySubject - Contact Feedback from the website";
  $message = "Name: $name \n\r"
           . "Phone Number: $phone \n\r"
           . "Message: $queryComments";
  
  $headers = "From: $name <$emailCheck>";

  mail($to, $subject, $message, $headers );
  echo '<div id="thankyoubox">';
  echo '<h2>Thank you</h2>';
  echo '<p>Thank you for submitting the contact us form. If you have requested information we will get back to you within 10 working days.</p>';
  echo '</div>';
      
}
?>
I have ran some xss security checks on it already, but I couldn't get anything to work (could be my lack of xss knowledge!).

Appreciate your help and constructive criticism :)



cheers
Jared
User avatar
mecha_godzilla
Forum Contributor
Posts: 375
Joined: Wed Apr 14, 2010 4:45 pm
Location: UK

Re: Can my simple php contact form be hijacked for anything

Post by mecha_godzilla »

Hi,

It might be worth posting your code again because of the way it's been parsed (which has made it difficult to read). I would also suggest splitting the code up into separate files (using include_once or require_once as appropriate) just so it's a bit easier to work with. Similarly, to make the page easier to maintain you could put the HTML sections in their own file and include them at runtime. If you do this then you need to make sure the extra files can't be accessed directly (I can tell you how to do this if you want to reorganise your scripts).

If you want to try re-posting the code it'll make it easier for other users to review it, but here are some common 'gotchas' associated with mail forms (I'm sure other users will be able to add to this list):

1. Not checking to see whether magic quotes is on or off - I tend to use the get_magic_quotes_gpc check rather than the runtime equivalent (in fact, it's been deprecated now anyway) as it was returning the wrong value. This is the script I use:

Code: Select all

if (!get_magic_quotes_gpc()) {
    $name = addslashes(strip_tags(trim($_POST['name'])));
} else {
    $name = strip_tags(trim($_POST['name']));
}
You can also use the mysql_escape_string() method if the data is being saved to a MySQL database. Going back to my point about the get_magic_quotes_runtime being deprecated, you should always check the main PHP site to see if any new (as in "new to you") functions you're using are appropriate for the version of PHP you've got.

2. Using exec() represents a risk - you need to make sure that the command is hard-coded so the user can't add their own cute little values to it, or escape it and add a new command. You should also have some error checking to make sure that if the command isn't run for some reason the server doesn't just dump a useful error message which might reveal what the script was trying to do before it quit. I recently had to test a login page connected to a MS SQL database - the person hadn't escaped the login and password fields properly so when I fed a load of escape strings in there the script exited and dumped some very useful information, but this information only appeared in Firefox and not Internet Explorer.

3. Although they can be faked, you should include some kind of referral check to make sure that the script is being called from the server rather than someone's hard drive - you could do this by capturing some information in a PHP session and then testing it to see if the results match when the script is run.

4. You could also capture the sender's IP address if you wanted and add it to a text log that gets updated whenever the script is run to see if anyone is trying to crack it (not a security measure as such but at least you might get some advance warning of mischief).

What you have done right is:

- specifically declared the POST values rather than allow them to automatically generated from the headers (this stops someone from guessing the other values you've used in the script and possibly replacing them)
- hard-coded the 'To:' address
- made sure the POST values are the expected length
- made sure the POST values don't contain any funny characters
- trimmed the POST values

I think that's all for the moment - it's not a comprehensive list but I think I've covered the obvious things. If you want any more input please let me know.

Mecha Godzilla
Post Reply