0

I want to make a account activation page in php. I tried with these code, but every time it show

Your verification link is invalid or expired

whenever the email id and secret key is in my sql database what's wrong with my code? please help

<?php 
    $hash = $_GET['hash'];
    $e = base64_decode($_GET['e']);
?>
<?php require_once("Connections.php");?>
<?php 
    mysql_select_db($database, $connection);
    $sql=mysql_query("SELECT FROM temp  WHERE email='$e' AND hashkey='$hash'");

    if(mysql_num_rows($sql)!=1) {
        echo "Your verification link is invalid or expired";
    }
    else {
        if ($sql=mysql_query("DELETE FROM temp  WHERE email='".$e."' AND hashkey='".$hash."'")) {
            echo '<script type="text/javascript">
                  register_2($e);
                  function register_2(){
                      alert("Hi your email"+m);
                  }
                  </script>
                 ';
        }
    }    
?>
Mosh Feu
  • 28,354
  • 16
  • 88
  • 135
SUJOY ROY
  • 29
  • 1
  • 1
  • 3
  • May be your SELECT query returns more than one row. Check that. – Rajdeep Paul Feb 04 '16 at 09:55
  • 1
    Where are `$e` and `$hash` variables coming from? – Hanky Panky Feb 04 '16 at 09:56
  • $e and $hash coming from GET methode – SUJOY ROY Feb 04 '16 at 09:57
  • But where? i don't see that in your code unless you use `register_globals`? – Hanky Panky Feb 04 '16 at 09:58
  • i checked i have only one row – SUJOY ROY Feb 04 '16 at 09:58
  • Please dont use the `mysql_` database extension, it is deprecated (gone for ever in PHP7) Especially if you are just learning PHP, spend your energies learning the `PDO` or `mysqli_` database extensions, [and here is some help to decide which to use](http://stackoverflow.com/questions/12859942/why-shouldnt-i-use-mysql-functions-in-php) – RiggsFolly Feb 04 '16 at 09:58
  • `echo "SELECT FROM temp WHERE email='$e' AND hashkey='$hash'";` and find out your error – Hanky Panky Feb 04 '16 at 09:58
  • 3
    "SELECT id, smt FROM" instead of "SELECT FROM" – Jonny Vu Feb 04 '16 at 09:59
  • Yeah what @VũTuấnAnh said is absolutely right, that should be it. That's the sort of guesswork we have to do when OP doesn't use error reporting – Hanky Panky Feb 04 '16 at 10:00
  • Unfortunately there are other errors in there as well – RiggsFolly Feb 04 '16 at 10:01
  • 1
    You are also WIDE OPEN to an [SQL Injection Attack](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) – RiggsFolly Feb 04 '16 at 10:05
  • @SUJOYROY You should really read up on SQL injection, it's trivial to verify any address by just passing a well chosen email string. For example, register with `me@bop.com` and validate with (base64 encoded) email `me@bop.com' OR ''='` – Joachim Isaksson Feb 04 '16 at 10:06
  • A small example about sql injection. They could activate your email by enter url like: `http://yourdomain.com/activatedlink?email=sample@sample.com' OR 1=1 #` – Jonny Vu Feb 04 '16 at 10:09
  • error_reporting(E_ALL); ini_set('display_errors', 1); add these two lines at the start of your code and edit your post with error reporting. So we can get what is the error there. And dont use mysql and sanitize your data. – Atilla Arda Açıkgöz Feb 04 '16 at 10:40

3 Answers3

0
<?php 
$hash = $_GET['hash'];

$e = base64_decode($_GET['e']);

?>


<?php require_once("Connections.php");?>
<?php 
mysql_select_db($database, $connection);
$sql=mysql_query("SELECT FROM temp  WHERE email='$e' AND hashkey='$hash'");
 if(mysql_num_rows($sql)<= 0)
   {
    echo "Your verification link is invalid or expired";
   }
else{
if ($sql=mysql_query("DELETE FROM temp  WHERE email='".$e."' AND hashkey='".$hash."'")){
        echo '<script type="text/javascript">
                    register_2($e);
                    function register_2(){
                        alert("Hi your email"+m);
                        }
              </script>
              ';
        }
    }

?>

just change !=1 to <=0
I do it

SUJOY ROY
  • 29
  • 1
  • 1
  • 3
0

Firstly don't use mysql_* functions they are depracated and are removed in php 7, use mysqli_* or pdo instead

so change

if(mysql_num_rows($sql)!=1)

to

$rowsCount = mysql_num_rows($sql);

if ($rowsCount === 0 || $rowsCount === false) {
    die "Your verification link is invalid or expired";
}

also your code is vulnerable to SQL INJECTION you should escape hash and also XSS register_2($e); here I can pass anything encoded in base64 via URL.

$hash = mysql_real_escape_string($_GET['hash']);

also this code

mysql_query("DELETE FROM temp  WHERE email='".$e."' AND hashkey='".$hash."'")

can be changed to

mysql_query("DELETE FROM temp  WHERE email='$e' AND hashkey='$hash'")

it's the same, but in the best case you should use prepared statements.

Other suggestions:

  1. Change echo to die;
  2. Remove else condition when you do 1.
  3. Your else condition does not check if query really removed rows it only check if query was executed, you should use mysql_affected_rows() to check if rows were deleted or remove this if condition.
Robert
  • 19,800
  • 5
  • 55
  • 85
-1

You should use the condition like this.

if(mysql_num_rows($sql) != NULL) { echo "Your verification link is invalid or expired"; }

StackUser
  • 5,370
  • 2
  • 24
  • 44
Hassan Shahbaz
  • 596
  • 1
  • 14
  • 38