Page 1 of 1
Am I a victim of Form mail injection?
Posted: Mon Feb 13, 2006 2:17 pm
by Luke
I am getting these emails from my online form and I cannot tell if I am being used by a spammer...
I'm not sure if anybody can tell from that, but if you can, please let me know. Thanks!
Posted: Mon Feb 13, 2006 2:38 pm
by feyd
what's the code that generates the output?
Posted: Mon Feb 13, 2006 2:42 pm
by Luke
Code: Select all
<?php
function checkEmail($email) {
if (!preg_match("/^([0-9a-zA-Z]([-.\w]*[0-9a-zA-Z])*@([0-9a-zA-Z][-\w]*[0-9a-zA-Z]\.)+[a-zA-Z]{2,9})$/", $email)) {
return false;
}
return true;
}
function MAIL_NVLP($fromname, $fromaddress, $toname, $toaddress, $subject, $message)
{
// Copyright ? 2005 ECRIA LLC, http://www.ECRIA.com
// Please use or modify for any purpose but leave this notice unchanged.
$headers = "MIME-Version: 1.0\n";
$headers .= "Content-type: text/plain; charset=iso-8859-1\n";
$headers .= "X-Priority: 3\n";
$headers .= "X-MSMail-Priority: Normal\n";
$headers .= "X-Mailer: php\n";
$headers .= "From: \"".$fromname."\" <".$fromaddress.">\n";
return mail($toaddress, $subject, $message, $headers);
}
class validateForm{
function validateForm($array){
$this->fname = $array['fname'];
$this->lname = $array['lname'];
$this->email = $array['email'];
$this->phone = $array['phone'];
$this->subject = $array['subject'];
$this->body = $array['body'];
switch($this->subject){
case "web design":
$this->recipient = "webdesigner@sierra-tech.com";
break;
case "web hosting":
$this->recipient = "webdesigner@sierra-tech.com";
break;
case "products":
$this->recipient = "marketing@sierra-tech.com";
break;
case "service":
$this->recipient = "bench@sierra-tech.com";
break;
case "ST systems":
$this->recipient = "bench@sierra-tech.com";
break;
case "training":
$this->recipient = "marketing@sierra-tech.com";
break;
case "cingular":
$this->recipient = "marketing@sierra-tech.com";
break;
case "technical":
$this->recipient = "bench@sierra-tech.com";
break;
default:
$this->recipient = "dean@paradisedirect.com";
}
}
function checkInfo(){
if(empty($this->fname)){
$errors['fname'] = "red";
}
if(empty($this->lname)){
$errors['lname'] = "red";
}
if(!empty($this->email)){
if(!checkEmail($this->email)){
$errors['email'] = "orange";
}
}
else{
$errors['email'] = "red";
}
if(empty($this->body)){
$errors['body'] = "red";
}
if(isset($errors)){
return $errors;
}
return false;
}
function mailInfo(){
$message = "Name: $this->fname $this->lname\nEmail Address: $this->email\nPhone: $this->phone\n\n$this->body";
$name = "$this->fname $this->lname";
if(MAIL_NVLP("$name", $this->email, "Sierra Tech", $this->recipient, "$this->subject - Sierra Tech Online Form", $message)){
header("Location: http://www.sierra-tech.com/contactus.php?sent=yes");
}
else{
header("Location: http://www.sierra-tech.com/contactus.php");
}
}
}
if($data = new validateForm($_POST)){
if($errors = $data->checkInfo()){
foreach($errors as $key => $val){
$urladd .= "$key=$val&";
}
foreach($_POST as $key => $val){
$urladd .= "p$key=$val&";
}
header("Location: http://www.sierra-tech.com/contactus.php?errors=yes&".$urladd);
}
else{
$data->mailInfo();
}
}
?>
Posted: Mon Feb 13, 2006 3:08 pm
by feyd
they are attempting spam, yes. You do have a few vulernabilities I can see:
- you allow $fromname to come in unchallenged (outside of checking it's length, in reality). One could inject headers through it.
- The subject is also a point of weakness similar to $fromname's
Posted: Mon Feb 13, 2006 3:30 pm
by Luke
so what would I do to "check" those?
Posted: Mon Feb 13, 2006 3:41 pm
by matthijs
Check for any line feeds and carriage returns. See
viewtopic.php?t=42190 for a thread with some example solutions and good links about the problem. I've had to deal with it last summer...
The simplest function is ctype_print()
Code: Select all
if (!ctype_print($string) )
{
return false;
}
or
Code: Select all
<?php
$from=$_POST["sender"];
if (eregi("\r",$from) || eregi("\n",$from)){
die("Why ?? ");
}
?>
See
http://securephp.damonkohler.com/index. ... _Injection
Posted: Mon Feb 13, 2006 3:45 pm
by Chris Corbyn

Moved to PHP Security (Could be useful reading to a few others

)
Posted: Mon Feb 13, 2006 4:40 pm
by Luke
Does this look ok?
Code: Select all
<?php
function checkEmail($email) {
if (!preg_match("/^([0-9a-zA-Z]([-.\w]*[0-9a-zA-Z])*@([0-9a-zA-Z][-\w]*[0-9a-zA-Z]\.)+[a-zA-Z]{2,9})$/", $email)) {
return false;
}
return true;
}
function MAIL_NVLP($fromname, $fromaddress, $toname, $toaddress, $subject, $message)
{
// Copyright ? 2005 ECRIA LLC, http://www.ECRIA.com
// Please use or modify for any purpose but leave this notice unchanged.
$headers = "MIME-Version: 1.0\n";
$headers .= "Content-type: text/plain; charset=iso-8859-1\n";
$headers .= "X-Priority: 3\n";
$headers .= "X-MSMail-Priority: Normal\n";
$headers .= "X-Mailer: php\n";
$headers .= "From: \"".$fromname."\" <".$fromaddress.">\n";
return mail($toaddress, $subject, $message, $headers);
}
class validateForm{
function validateForm($array){
$this->fname = $array['fname'];
$this->lname = $array['lname'];
$this->email = $array['email'];
$this->phone = $array['phone'];
$this->subject = $array['subject'];
$this->body = $array['body'];
switch($this->subject){
case "web design":
$this->recipient = "webdesigner@sierra-tech.com";
break;
case "web hosting":
$this->recipient = "webdesigner@sierra-tech.com";
break;
case "products":
$this->recipient = "marketing@sierra-tech.com";
break;
case "service":
$this->recipient = "bench@sierra-tech.com";
break;
case "ST systems":
$this->recipient = "bench@sierra-tech.com";
break;
case "training":
$this->recipient = "marketing@sierra-tech.com";
break;
case "cingular":
$this->recipient = "marketing@sierra-tech.com";
break;
case "technical":
$this->recipient = "bench@sierra-tech.com";
break;
default:
$this->recipient = "dean@paradisedirect.com";
}
}
function checkInfo(){
if(!empty($this->fname)){
if(!ctype_print($this->fname)){
$errors['fname'] = "orange";
}
}
else{
$errors['fname'] = "red";
}
if(!empty($this->lname)){
if(!ctype_print($this->lname)){
$errors['lname'] = "orange";
}
}
else{
$errors['lname'] = "red";
}
if(!empty($this->email)){
if(!checkEmail($this->email)){
$errors['email'] = "orange";
}
}
else{
$errors['email'] = "red";
}
if(!empty($this->subject)){
if(!ctype_print($this->subject)){
$errors['subject'] = "orange";
}
}
else{
$errors['subject'] = "red";
}
if(empty($this->body)){
$errors['body'] = "red";
}
if(isset($errors)){
return $errors;
}
return false;
}
function mailInfo(){
$message = "Name: $this->fname $this->lname\nEmail Address: $this->email\nPhone: $this->phone\n\n$this->body";
$name = "$this->fname $this->lname";
if(MAIL_NVLP("$name", $this->email, "Sierra Tech", $this->recipient, "$this->subject - Sierra Tech Online Form", $message)){
header("Location: http://www.sierra-tech.com/contactus.php?sent=yes");
}
else{
header("Location: http://www.sierra-tech.com/contactus.php");
}
}
}
if($data = new validateForm($_POST)){
if($errors = $data->checkInfo()){
foreach($errors as $key => $val){
$urladd .= "$key=$val&";
}
foreach($_POST as $key => $val){
$urladd .= "p$key=$val&";
}
header("Location: http://www.sierra-tech.com/contactus.php?errors=yes&".$urladd);
}
else{
$data->mailInfo();
}
}
?>
Posted: Mon Feb 13, 2006 4:53 pm
by josh
From first glance, I notice your regex is not RFC compliant
Posted: Mon Feb 13, 2006 5:07 pm
by Luke
jshpro2 wrote:From first glance, I notice your regex is not RFC compliant
I don't know what that is. Is it really important?
Posted: Mon Feb 13, 2006 5:10 pm
by josh
It means that you could block out people that actually have a valid email.
Check out Roja's validator
http://svn.gna.org/viewcvs/blacknova/tr ... iew=markup
Posted: Tue Feb 14, 2006 1:55 am
by Chris Corbyn
yep... that would block out my
me@mysite.co.uk domains and also I actually have a work email address which is
me@my-site-name.manchester.sch.uk
thanks!
Posted: Tue Feb 14, 2006 1:48 pm
by goodtimetribe
That's some yummy goodness. I appreciate that one

Re: thanks!
Posted: Tue Feb 14, 2006 3:58 pm
by Roja
goodtimetribe wrote:That's some yummy goodness. I appreciate that one

Very glad you like it. Credit where it's due - the power came from the original book.. Thats just a php translation of that power.