ik zie dat ik een behoorlijk veiligheidslek heb op mijn website.
Het gaat om de volgende site: www.jackmaessen.nl
Je kunt inloggen met username:test en password: pass
Eenmaal ingelogd, klik in de navigatie op "My account"
Kijk vervolgens in de url en zie dat de username in de url wordt aangeroepen
(http://www.jackmaessen.nl/userinfo.php?user=test)
er is dus nu vrij gemakkelijk de info van andere leden te raadplegen door bv. in de url "test" te vervangen door "piet" of een andere naam. Dit mag natuurlijk nooit gebeuren.
Het script dat de info parsed ziet er als volgt uit:
<!-- MY ACCOUNT INFO -->
<?php
/**
* UserInfo.php
*
* This page is for users to view their account information
* with a link added for them to edit the information.
*
* Updated by: The Angry Frog
* Last Updated: October 26, 2011
*/
if (!isset($_GET['user'])) {
header("Location: ".$config['WEB_ROOT'].$config['home_page']);
}
?>
<?php
/* Requested Username error checking */
$req_user = trim($_GET['user']);
if(!$req_user || strlen($req_user) == 0 ||
!preg_match("/^[a-z0-9]([0-9a-z_-\s])+$/i", $req_user) ||
!$database->usernameTaken($req_user)){
die("Username not registered");
}
/* Logged in user viewing own account */
if(strcmp($session->username,$req_user) == 0){
echo "<h6><br>My Account</h6>";
}
/* Visitor not viewing own account */
else{
echo "<h6>User Info</h6>";
}
/* Display requested user information - add/delete as applicable */
$req_user_info = $database->getUserInfo($req_user);
/* Username */
echo "<b> Username: ".$req_user_info['username']."</b><br />";
/* Email */
echo "<b> Email:</b> ".$req_user_info['email']."<br />";
/**
* Note: when you add your own fields to the users table
* to hold more information, like homepage, location, etc.
* they can be easily accessed by the user info array.
* $session->user_info['location']; (for logged in users)
* $req_user_info['location']; (for any user)
*/
/* If logged in user viewing own account, give link to edit */
if(strcmp($session->username,$req_user) == 0){
echo '';
}
/* Link back to main */
/*echo "<br>Back To [<a href='".$config['WEB_ROOT'].$config['home_page']."'>Main</a>]<br>";*/
?>
<!-- EINDE MY ACCOUNT INFO -->
Niet met GET parameters werken. Hoe een inlog systeem hoort te werken:
- Je logt in, nadat alles is gecontroleerd maak je een random hash aan (met bijv. [php]uniqid[/php] + een sha functie in een gegeven van de gebruiker), deze sla je op in de database en in een sessie.
- Op elke volgende pagina kijk je of deze hash nog steeds hetzelfde is (door de sessie met een db te controleren), zo ja => gebruiker ingelogd, zo nee => gebruiker uitgelogd.
- Op elke volgende pagina kijk je of deze hash nog steeds hetzelfde is (door de sessie met een db te controleren), zo ja => gebruiker ingelogd, zo nee => gebruiker uitgelogd.
Wouter, maak je dan bij iedere pagina een database request?
Je kunt ook in de sessie opslaan dat iemand is ingelogd ($logged_in=true). Of is dat qua veiligheid te risicovol?
<?php
if(strcmp($session->username,$req_user) == 1){
echo "alert("This is not your profile!")";
header(Location: http://www.jackmaessen.nl/userinfo.php?user=$session->username);
}
?>
Ik weet niet of dat handig is, maar in ieder geval krijgt u een alert met het bericht "This is not your profile!" en wordt u direct doorgestuurd naar uw eigen profiel. ;)
de oplossing van marco werkt niet, hij toont nog steeds de user info bij naamsverandering, maar ik heb het als volgt opgelost:
Als usernaam variable gelijk is aan requested user, toon de info; anders ga naar userinfo.php waarna de cyclus opnieuw doorlopen wordt en died bij "requested username error checking"
<?php
/* Requested Username error checking */
$req_user = trim($_GET['user']);
if(!$req_user || strlen($req_user) == 0 ||
!preg_match("/^[a-z0-9]([0-9a-z_-\s])+$/i", $req_user) ||
!$database->usernameTaken($req_user)){
die("<br /></br />Username not registered or you don't have permission to view this information");
}
/* Logged in user viewing own account */
if(strcmp($session->username,$req_user) == 0){
echo "<h6><br>My Account</h6>";
}
/* Visitor not viewing own account */
else{
header('Location: userinfo.php');
/*echo "<h6><br /><br />User Info</h6>";*/
}
/* Display requested user information - add/delete as applicable */
$req_user_info = $database->getUserInfo($req_user);
/* Username */
echo "<b> Username: ".$req_user_info['username']."</b><br />";
/* Email */
echo "<b> Email:</b> ".$req_user_info['email']."<br />";
?>
Wouter, maak je dan bij iedere pagina een database request?
Dat doe je in principe toch al.
Je kunt ook in de sessie opslaan dat iemand is ingelogd ($logged_in=true). Of is dat qua veiligheid te risicovol?
En dan pas ik lokaal even die sessie aan zodat ik al ben ingelogd terwijl ik niks heb ingevoerd? Wat je natuurlijk eerst doet is kijken of die sessie bestaat, zo niet dan hoef je niet verder te gaan met de DB, zoja dan kijk je of de hash in de DB nog steeds geldig is, of dat hij bijv. zelf een random hash heeft ingevuld of misschien wel gekopieerd van een andere gebruiker van die website.
<?php
if(strcmp($session->username,$req_user) == 1){
echo "alert("This is not your profile!")";
header(Location: http://www.jackmaessen.nl/userinfo.php?user=$session->username);
}
?>
Ik weet niet of dat handig is, maar in ieder geval krijgt u een alert met het bericht "This is not your profile!" en wordt u direct doorgestuurd naar uw eigen profiel. ;)
Dit kan natuurlijk niet. Bij de header krijg je dan de foutmelding 'Headers already sent'. Als je een redirect doet kan je niet ook nog gewone output versturen.
Wouter, maak je dan bij iedere pagina een database request?
Dat doe je in principe toch al.
Waarom zou je dat doen?
[/quote]
Om te controleren of je nog wel werkelijk de user bent? Tokens opslaan in je db heeft ook heel veel extra handige features:
- je kan leden aan jouw kant uitloggen
- leden kunnen eerdere sessies aan hun kant uitloggen
- je kan makkelijk online leden ophalen
