Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Any major security risks? And please don't get angry over my novice log system.

<?php
   function makesalt($lg)
    {
    $chars = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789!@#$%^&*()-';  
    $sz = strlen( $chars );
    $str = '';
    for( $i = 0; $i < $lg; $i++ ) {
        $str .= $chars[ rand( 0, $sz - 1 ) ];
    }
    return $str;
  }
  function duelsha($th)
  {
      $tem = hash('sha512', $th);
      $tem2 = md5($tem);
      return hash('sha256', $tem2);
  }
  function shasalt($ht)
  {
      $salt = makesalt(30);
      return duelsha($ht . $salt) . '|' . $salt;
  }
  function mysqlsan($ss)
  {
      if (get_magic_quotes_gpc()) $ss = stripslashes($ss);
      return mysql_real_escape_string($ss);
  }
  function htent($st)
  {
      return htmlentities($st);
  }
  function sqlhtml($sm)
  {
       return htent(mysqlsan($sm));
  }
  function logs($tl)
  {
    $fh = fopen("server.log", 'w') or die("File error");
    fwrite($fh, $tl) or die("File error");
    fclose($fh);
  }
?>
share|improve this question

1 Answer 1

up vote 4 down vote accepted

You home brewed security hashes and such are a big NO. Please, check here and here for a good read regarding that. Also, please do not hash a hash, that can lead to collisions and should be avoided! Using a method such as password_hash() creates a salt for you, therefore you shouldn't have to make one on your own.

You're using mysql_real_escape_string() which is not the way to go. If possible, move away from that and onto mysqli or PDO.

And then your function htent() is sort of redundant. You don't have anything else in the function, so it shouldn't be needed.

share|improve this answer
    
Thank you. You feedback is greatly appreciated! –  xv435 Feb 18 '14 at 3:51

Your Answer

 
discard

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.