0

very new to PHP and working on a mailing list with double opt-in. I found this code here: http://www.sitepoint.com/forums/showthread.php?632689-Guide-to-creating-a-double-opt-in-email-list&s=560d4d5d106fde7ccc1a53b507c436cd&p=4350152&viewfull=1#post4350152 and made it work until I click on the link in the e-mail, for some reason it doesn't update the status in the database to "confirmed" (it says "Sorry, account unvalidated").

I can't find the error with my basic knowledge, would appreciate some help! Thanks a lot JD

signup.php:

<?php
if(true === array_key_exists('submit', $_POST))
{
require("connect.php");

$rResult = mysqli_query($conn,
    sprintf(
        "INSERT INTO newsletter2 (email, status)VALUES('%s', 'pending')",
        $_POST['email']
    )
);
mail(
    $_POST['email'],
    'Subscriber Confirmation',
    sprintf(
        'Thankyou, please visit http://****.com/newsletter2/confirmSignUp.php?key=%s to confirm your subscription.',
        sha1($_POST['email'])
    ),
    null,
    null
);
echo 'Thankyou, please check the provided email for a link to confirm your subscription.';
exit;
}
?>

confirmSignUp.php:

<?php
if(true === array_key_exists('key', $_GET))
{
require("connect.php");

$rResult = mysqli_query($conn,
    sprintf(
        "UPDATE newsletter2 SET status = 'confirmed' WHERE email = SHA1('%s')",
        $_GET['key']
    )
);
if(mysqli_affected_rows($conn) > 0)
{
    echo 'Thankyou, email confirmed';
}
else
{
    echo 'Sorry, account unvalidated.';
}
exit;
}
?>
JD26
  • 47
  • 4
  • 1
    You need to put quotes around string values – Patrick Q Jan 03 '18 at 22:21
  • 1
    Not related to the question. You dont need to do `(true === array_key_exists('submit', $_POST))` just use `(array_key_exists('submit', $_POST))` – SunriseM Jan 03 '18 at 22:22
  • String literal values need to be quoted – Sean Jan 03 '18 at 22:23
  • 1
    Your script is wide open to [SQL Injection Attack](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) Even [if you are escaping inputs, its not safe!](http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string) Use [prepared parameterized statements](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php) – RiggsFolly Jan 03 '18 at 22:24
  • 1
    Why are you expecting `sha1($_POST['email']` to be that same as `$_POST['email']` – RiggsFolly Jan 03 '18 at 22:27
  • And in the `confirm.php` you are `sha1()`ing the `sha1`'d hash again. Basically it is not finding the originally inserted row because you are not using the same value in the `email` value as you stored it with – RiggsFolly Jan 03 '18 at 22:30
  • _Small Note_ SHA1() is a One Way Hash ... emphasising the **One Way** bit – RiggsFolly Jan 03 '18 at 22:30
  • Thank you for all the comments! I'm afraid I don't really understand the error though :( never used SHA, keys etc before. What do I need to put in the Where clause so it finds the row? – JD26 Jan 03 '18 at 22:40
  • As for safety, don't worry, it's for a fictional project only. Want to make it work first, then I might try some advanced things with it – JD26 Jan 03 '18 at 22:42

1 Answers1

0

signup.php

$rResult = mysqli_query($conn,
    sprintf(
        "INSERT INTO newsletter2 (email, status, authkey)VALUES('%s', 'pending', '%s')",
        $_POST['email'],
        sha1($_POST['email']) 
    )
);

confirmSignUp.php:

$rResult = mysqli_query($conn,
    sprintf(
        "UPDATE newsletter2 SET status = 'confirmed' WHERE authkey = '%s'",
        $_GET['key']
    )
);
Zia Ur Rehman
  • 116
  • 5