Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I've created a logging system with one-use passwords (sent via SMS API). What I would like to know is if it's "safe". I know it is hard to estimate safety but I am curious if it is safer than regular password. I would appreciate any advice to improve following code.

Scheme is:

Logging:

Loggin at a webpage -> Ask external server for password -> Store the password in session var and when form is sent compare it with the sent one

External script:

When asked with proper apikey -> give random password and send it via SMS API

The code is below:

External script:

    <?php 

   $apiKey='foobar123';

   if ($_GET['api']!=$apiKey) {

       header('HTTP/1.0 404 Not Found');
       die();
   }

    $ile=12; //char number

    $onepass='';
    for($i=0;$i<$ile;$i++)
    {
        $k=rand(1,10);
        $onepass.=$letter[$k];
    }
    if (!isset($_POST['ip'],$_POST['ip'])) {echo 'error'; die();}
    $ip=$_POST['ip'];
    $www=$_POST['www'];
    echo $onepass;

    include('sender.php'); //sending API

    ?>

Logging script (at page foo.com)

    <?php 
    session_start();

    $sessi='1231dsahsda8'; 
    $www="foo.com";
    $ip=$_SERVER['REMOTE_ADDR'];

    if(isset($_GET['destroyer'])){
        if($_GET['destroyer']=='yes'){
            session_destroy(); echo 'Logged out - <a href="?relog">Log in again</a>'; die();
        }
        if($_GET['destroyer']=='no'){
            session_destroy(); echo 'New pass has been sent!'; session_start();
        }
    }

    if (isset($_POST['password'])){
        if($_POST['password']==$_SESSION['pass']) $_SESSION[$sessi]="log";
        else echo'Wrong Password <a href="?destroyer=no">Send again</a>';
    }

    if ($_SESSION[$sessi]!="log") { 
        if(!isset($_SESSION['pass'])){
            $adres='http://www.foobar.com/index.php?api=foobar123';
            $c = curl_init();
            curl_setopt($c, CURLOPT_URL, $adres);
            curl_setopt($c, CURLOPT_POST, 1);
            curl_setopt($c, CURLOPT_POSTFIELDS, "ip=$ip&www=$www"); 
            curl_setopt($c, CURLOPT_RETURNTRANSFER, 1); /
            $dane=curl_exec($c); 
            curl_close($c); 
            $_SESSION['pass']=$dane;
            echo 'Password has been sent';
        }
        echo'<p>Log in</p><form action="?a" method="post"><input type="password" name="password" value=""/><input type="submit" value="Login" /></form>';
    }
    else{//authorised
         echo 'Logged <p><a href="?destroyer=yes">Logout</a></p'; 
    }

    ?>
share|improve this question
Why have you decided to split this up into two scripts? It seems that everything taking place in the "api" script could replace the curl stuff in the log in script (not "logging"). – Michael Zedeler Nov 18 '12 at 21:33
in the future i would like to store in one place phone number for each user and have one account for few sites - that's why I would like to store it in another place – Niedam Wam Nov 20 '12 at 18:00

Know someone who can answer? Share a link to this question via email, Google+, Twitter, or Facebook.

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Browse other questions tagged or ask your own question.