confirmation page

Coding Critique is the place to post source code for peer review by other members of DevNetwork. Any kind of code can be posted. Code posted does not have to be limited to PHP. All members are invited to contribute constructive criticism with the goal of improving the code. Posted code should include some background information about it and what areas you specifically would like help with.

Popular code excerpts may be moved to "Code Snippets" by the moderators.

Moderator: General Moderators

Post Reply
shivam0101
Forum Contributor
Posts: 197
Joined: Sat Jun 09, 2007 12:09 am

confirmation page

Post by shivam0101 »

This is the code written for confirming members through email.

Code: Select all

<?php
require_once('general/require_once.php');

$confirm_code=$_GET['cc'];

if(!empty($confirm_code))
{
	$query_check=mysql_query("SELECT * FROM members WHERE confirm_code='$confirm_code' AND confirm_flag='YES'");
	if(mysql_num_rows($query_check) == 1)
	{
            
	   $fetch_det=mysql_fetch_array($query_check);
	   $member_id=$fetch_det['member_id'];
	   session_start();
       $_SESSION['member_id']=$member_id;
	   header("Location: ".SITE_URL);
	}


    elseif(mysql_num_rows(mysql_query("SELECT * FROM members WHERE confirm_code='$confirm_code' AND confirm_flag !='YES'"))==1)
    {
      $date=GetDateTime();
      $query_update_confirm=mysql_query("UPDATE members SET confirm_flag='YES', confirmation_date='$date' WHERE confirm_code='$confirm_code'");
      
      if($query_update_confirm)
      {
	  	$query_det=mysql_query("SELECT * FROM members WHERE confirm_code='$confirm_code' AND confirm_flag='YES'");
      	if(mysql_num_rows($query_det)==1)
      	{
         	$fetch_det=mysql_fetch_array($query_det);
	     	$member_id=$fetch_det['member_id'];
	     	session_start();
         	$_SESSION['member_id']=$member_id;
	     	header("Location: ".SITE_URL);
	  	}
      }
    }
    else
    echo 'Invalid confirmation';
   
}


?>
If there is any improvements that has to be made in the above code, please let me know.

Thanks
User avatar
Chris Corbyn
Breakbeat Nuttzer
Posts: 13098
Joined: Wed Mar 24, 2004 7:57 am
Location: Melbourne, Australia

Post by Chris Corbyn »

Read up on "SQL Injection Attacks" ;)
User avatar
CoderGoblin
DevNet Resident
Posts: 1425
Joined: Tue Mar 16, 2004 10:03 am
Location: Aachen, Germany

Post by CoderGoblin »

Redirections using header should always be followed by exit;. Even though the header is sent the program continues until it finishes. If you ever have things outside tthe if structure they will be processed. which could lead to strange side effects.

You may want to look at mysql_real_escape_string which shows an SQL Injection.
shivam0101
Forum Contributor
Posts: 197
Joined: Sat Jun 09, 2007 12:09 am

Post by shivam0101 »

If you ever have things outside tthe if structure they will be processed. which could lead to strange side effects.
Can you give more information?
User avatar
CoderGoblin
DevNet Resident
Posts: 1425
Joined: Tue Mar 16, 2004 10:03 am
Location: Aachen, Germany

Post by CoderGoblin »

A brief example (not bothering with too much correctness)

Code: Select all

<?php

if (empty($_GET['insert']) {
    $_SESSION['message']='We aren\'t doing anything';
    header("Location: http://www.mypage.com/page2.php");
}
usleep(5000000);
$_SESSION['message']='Well you processed the form';
?>
<!-- PLACE HTML HERE POSSIBLY -->
I would expect in a situation like this, you will wait for 5 seconds for the jump to page2.php and the $_SESSION value will be the second one even if you have a $_GET['insert'] value. That's bad enough but consider the dangers if you modify information in a database outside of the IF. Because this is an unexpected "side effect" it will be difficult to track down as it should not be getting to where you set the 2nd $_SESSION. Even if you don't set or change anything outside the IF, the program flow will go to the end wasting processing time.
Post Reply