Gustav Wiberg wrote:

All you guys, please comment if the code is well or bad written and why... :-)

Since you asked, a few things popped out from a security perspective, though I didn't read through your code very thoroughly....


<?php

function chkIfPasswordTrue($un, $pw, $typeUser) {

//Make username and password in-casesensitive
//
$un = strtolower($un);

$pw = strtolower($pw);


Why limit your usernames/passwords to lower case? You've just made them significantly easier to brute force.




$sql = $sql . "SELECT IDAnvandare FROM tbanvandare WHERE";

$sql = $sql . " Anvandarnamn=" . safeQuote($un) . " AND";

$sql = $sql . " Losenord=" . safeQuote($pw) . " AND";


Where is your safeQuote() function coming from? From what I can see of your code you aren't doing any testing against the username and password before they are used as part of your SQL query. Sure would suck to have an unauthenticated user drop or otherwise muck with your db!



if (isset($_REQUEST["frmUsername"])) {

$un = $_REQUEST["frmUsername"];

If you're going to use $_REQUEST you might as well just turn on register globals (no, don't!).

If you're expecting a post look for a $_POST, if you're expecting a get look for a $_GET. Ditto with cookies. You really need to know where your variables are coming from if you want a measure of security.

- Ben

--
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to