Simple Form Broken with PHP 5

Morbius

My host doesn't allow for PHP 4 anymore and now a simple contact form I have doesn't work. I haven't been keeping up with the changes to PHP so I have no idea what changed.

I have a form using freecap verification. This is the code in the header. The form itself is standard stuff. I'm guessing it's something simple... When I hit submit it just reloads the page. No errors, nothing. It doesn't do anything.

session_start();

if($submit == 1 && !empty($_SESSION['freecap_word_hash'])) {
	//Setup error variables
	$feederror1 = 0;
	$feederror2 = 0;
	$feederror3 = 0;
	$feederror4 = 0;

//Check if any of the form variables are blank
if (!$realname) {$feederror1++;}
if (!$email) {$feederror2++;}
if (!$message) {$feederror3++;}

//Email Validation
	if (!$email) {$feederror2++;} //Is it empty?
	//Is it in a valaid format?
	if(!eregi("^[_a-z0-9-]+(\.[_a-z0-9-]+)*@[a-z0-9-]+(\.[a-z0-9-]+)*(\.[a-z]{2,3})$", $email)) {		
		$feederror4++;
	}

//Check is the verification box is empty
	if(empty($_POST['word'])) { $word_ok = "false"; }

if ($feederror1==1||$feederror2==1||$feederror3==1||$feederror4==1||$word_ok=="false") {
	//We have errors
} else {

	if($_SESSION['hash_func'](strtolower($_POST['word']))==$_SESSION['freecap_word_hash'])
	{
		// reset freeCap session vars
		// cannot stress enough how important it is to do this
		// defeats re-use of known image with spoofed session id
		$_SESSION['freecap_attempts'] = 0;
		$_SESSION['freecap_word_hash'] = false;

$message = stripslashes($message);
$msg = "Sender's Name: $realname\n";
$msg .= "Sender's E-mail: $email\n";
$msg .= "Message: $message\n";

$mailheaders = "From: $email\n";
$mailheaders .= "Reply-To: $email\n\n";

mail("****@******.com", "*********", $msg, $mailheaders);
$sent = 1;

unset($realname);
unset($email);
unset($message);

		$word_ok = "yes";
	} else {
		$word_ok = "no";
	}
};
}

bradgrafelman

Where is $submit ever defined? Or $realname? Or... (et al.).

It appears as though your script(s) depended upon the register_globals technique, something that has been deprecated for quite some time and even removed from PHP6. See this manual entry for more information about the security warning against using register_globals here: [man]security.globals[/man].

What you need to do is to start using the superglobals, such as $_POST to access incoming POST'ed data. See this manual entry for more information about these superglobals: [man]variables.superglobals[/man].

Weedpacket

Speaking of PHP6, you might also want to upgrade to using preg_match() instead of eregi().

bradgrafelman wrote:
What you need to do is to start using the superglobals, such as $POST to access incoming POST'ed data.

For example, as in the line:

        if(empty($_POST['word'])) { $word_ok = "false"; }

Morbius

I didn't include the form part as that's just normal code.

I'm just now finding out about this $_post stuff. This is all new to me and a bit frustrating. Guess I'm way old-school PHP.

So I have to use $_post for every form variable now? Yeesh. That's a lot of work and re-coding of my other sites.

Just to get me started can someone show me how this would look with the code I given?

I tried this: if($POST['submit'] == 1 && !empty($SESSION['freecap_word_hash'])) {

to get the ball rolling and that doesn't seem to work.

Also would it be if (!$post['realname']) {$feederror1++;} or if(empty($POST['realname'])) {$feederror1++;}

This is going to be a nightmare...

Morbius

Weedpacket;10901481 wrote:
Speaking of PHP6, you might also want to upgrade to using preg_match() instead of eregi().
For example, as in the line:
        if(empty($_POST['word'])) { $word_ok = "false"; } 

Oh man... this is just a contact form... I have a huge PHP CMS script I wrote that I've been using for years... I can just imagine all the things wrong with that...

I need a PHP old to PHP new converter...

bradgrafelman

The '== 1' part may be off depending upon the HTML of the code. Also note that $post is not a superglobal, whereas $POST is.

You could also simplify the code a bit by just having a single $error variable set to false, and if any error occurs (since you don't seem to discriminate between the types of errors in this script) simply set it to true. Other methods include making a $errors variable into an array of error messages.

Also note that it is a very, very bad practice to set the "From" header to the user's supplied e-mail address. This effectively makes your application an open proxy and will most likely get your server flagged on many RBLs (realtime block lists).

Morbius

bradgrafelman;10901489 wrote:
The '== 1' part may be off depending upon the HTML of the code. Also note that $post is not a superglobal, whereas $POST is.

It's not the == 1, that's from a hidden field. I tested it without the rest of that line and it worked. So it's something with the !empty($_SESSION['freecap_word_hash'])

bradgrafelman;10901489 wrote:
You could also simplify the code a bit by just having a single $error variable set to false, and if any error occurs (since you don't seem to discriminate between the types of errors in this script) simply set it to true. Other methods include making a $errors variable into an array of error messages.

I do actually have an error echo for each field and error type it's just not shown in the code I gave. Figured it wasn't important.

bradgrafelman;10901489 wrote:
Also note that it is a very, very bad practice to set the "From" header to the user's supplied e-mail address. This effectively makes your application an open proxy and will most likely get your server flagged on many RBLs (realtime block lists).

This form is emailed to me and me only. So that shouldn't be an issue (I don't think). But are you saying you can't use From: anymore? How are you supposed to know what email address it came from? This security stuff is getting out of hand.

If I could figure out that session thing I think I could get the rest working. Has anyone else used FreeCap varification? Maybe it's out of date for PHP5.

bradgrafelman

I have no idea about FreeCap verification - you might try doing a print_r() on the $_SESSION to see what all is stored.

Morbius wrote:
This form is emailed to me and me only.

Well, as far as you know. Standard bot scripts could easily do an e-mail header injection and "Cc" or "Bcc" tons of e-mails they have on file, causing you to be the source of spam.

Morbius wrote:
But are you saying you can't use From: anymore? How are you supposed to know what email address it came from?

Use the Reply-To header and set that to "name <email>" if you'd like, but the "From" part should be a valid e-mail on your own domain, otherwise it looks like you're trying to relay messages/forge the "From" header.

Morbius

bradgrafelman;10901494 wrote:
Use the Reply-To header and set that to "name <email>" if you'd like

If you'd like? Is there a better way? What should I actually use that won't be frowned on?

As for the contact form, I got it to work. Thanks for all your help.

I also learned that you can't just use ! for empty, you actually have to type out empty now... yeesh. Except this !eregi() still seems to work. Though, as posted above I need to change that now too. I just added more lines of code than before. This is efficient?

BTW I tried !preg_match() and it gave me a link in my header when I submitted the form? I guess that whole line is wrong, or does it want empty instead of ! ?