Invalid Input???

[ts01]

Hi there,

I have the following scrippt for emailing a form which seems to work great, except when a user tries to put a return in the comments field. When they ttry and create a new line and try to send the form in pops up with an invalid input error on a seperate page.

I don't have much knowledge in php and found this script in another thread, invalid input is in the script but i don't know what it means and why it pops up on puting a new line in the comments.


Code>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>

<?
function checkOK($field)
{
if (eregi("\r",$field) || eregi("\n",$field)){
die("Invalid Input!");
}
}

$name=$_POST['name'];
checkOK($name);
$email=$_POST['email'];
checkOK($email);
$comments=$_POST['comments'];
checkOK($comments);
$to="site@domain.co.uk";
$message="Name: $name\n\nMessage: $comments\n\nE-mail Addres: $email";
if(mail($to,"Comments From Portfolio Website",$message,"From: $email\n")) {
header("Location: thankyou.htm");

} else {
header("Location: Error.htm");

}
?>

>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>..

Incidently even if it does come up with an error, can you see why it doesnt redirect to the error.htm?

Any help on both issues would be great,

Thanks

Trev

[SpectreReturns]

Poorly written code. There are more } than {, which is why the error page isn't redirected. The check is done if (eregi("\r",$field) || eregi("\n",$field)){ here, checking if a newline or cariage return exist in the message. This is done so that extra headers cant be injected into the mail function, which causes security holes.

[ts01]

Thankyou for the info, but looking at the code i can't see what i would need to take out of the code to make it redirect properly. At the top where you mentioned, there are 2 { and 2 }. Sorry for the confusion.

Thanks

Trev

[sitehatchery]

If you REALLY want to grant cariage returns and igore the possibility of security loopholes, and without creating a debugging headache, you could just take the following lines out of your function:

if (eregi("\r",$field) || eregi("\n",$field)){
die("Invalid Input!");

[ts01]

Right, so i need this bit of code to keep the code that little bit more secure, thats fine, so to avoid this message in the first place, can anyone tell my why on my form i can't have a new line (return) in the text as it just throws up this message.

Thanks for your continued help.

Trev

[SpectreReturns]

\r and \n are used in combination with (or in absence of) each other to make new lines on different systems. Since you're stopping mail based on if the message contains either of those characters, putting them in will kill the script.

The error page would only be called if the mail can't be sent but everything is nice and valid.

The code is horribly mangled, and I don't think it deserves to be fixed, but here's how it'd work if you don't need any of that fancy "security hole fixing" stuff (and for the love of god, please indent and space propperly).

$message = "Name: " . $_POST['name'] . "\n\nMessage: " . $_POST['comments'] . "\n\nE-mail Address: " . $_POST['email'];

if (mail("portfolio@tsherwin.co.uk",
"Comments From Portfolio Website",
$message, "From: " . $_POST['email'] . "\n"
)) {
header("Location: thankyou.htm");
} else {
header("Location: Error.htm");
}

[ts01]

Thanks Spectrereturns,

That script works great, and fixes the problem. But does that mean i have security issues in the script. What are the implications, what can people do to exploit the script.

Thanks

Trev

[SpectreReturns]

People can inject headers into your mail, sending it to huge amounts of people and such. You can prevent this by sending the mail like this:

function safeguard(&$string) {
$string = str_replace(array("\r", "\n"), null, nl2br($string));
return $string;
}

$message = "Name: " . safeguard($_POST['name']) . "\n\nMessage: " . safeguard($_POST['comments']) . "\n\nE-mail Address: " . safeguard(POST['email']);

if (mail("portfolio@tsherwin.co.uk",
"Comments From Portfolio Website",
$message,
"From: " . $_POST['email'],
"Content-type: text/html"
)) {
header("Location: thankyou.htm");
} else {
header("Location: Error.htm");
}

This will now replace all newlines with html breaks and then send the email as html, mimicking the same behavior but removing the ability to inject data.

[ts01]

Right i see,

Well i added that part to the script and it seems to work great. So by doing this the line break issue has been resolved, and security hole is more secure, and i have learned a little more.

Thanks for your help.

Trev