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 -->
nav opmerking Erwin: dan kan ik hem ook eindigen met een die command, maar dan heb ik weer niet de mogelijkheid om de footer te embedden eronder
Voor de volledigheid, dit is mijn userinfo.php:
<!-- PHP LOGIN GEDEELTE -->
<?php
/*
* Main.php
*
* This is an example of the main page of a website. Here users will be able to login.
* However, like on most sites the login form doesn't just have to be on the main page,
* but re-appear on subsequent pages, depending on whether the user has logged in or not.
*
* Originally written by: Jpmaster77 a.k.a. The Grandmaster of C++ (GMC)
* Last Updated by the Angry Frog: October 19, 2011
*/
include("include/session.php");
global $database;
$config = $database->getConfigs();
?>
<?php
/**
* User has already logged in, so display relavent links, including
* a link to the admin center if the user is an administrator.
*/
if($session->logged_in){
echo "<h1></h1>";
echo "<a href=\"index.php\">My Files</a> "
."<a href=\"useredit.php\">Edit Account</a> ";
if($session->isAdmin()){
echo "<a href=\"admin/index.php\">Admin Center</a> ";
}
echo "<a href=\"upload.php\">Upload</a> "
."<a href=\"process.php\">Logout</a>";
/* $pagina = "users/".$session->username."/index.php";
include $pagina;*/
?>
<!-- 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']);
}*/
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("<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: forbidden.html');*/
die("<br /></br />xxxUsername not registered or you don't have permission to view this information");
/*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 />";
/**
* 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>";*/
?>
<?php
}
?>
<!-- EINDE MYACCOUNT INFO -->
nav opmerking Erwin: dan kan ik hem ook eindigen met een die command, maar dan heb ik weer niet de mogelijkheid om de footer te embedden eronder
Ik begrijp even niet hoe je mijn opmerking naar dat vertaalt.
Een exit() (of die()) gebruik je om ervoor te zorgen dat er geen verdere output meer komt na een redirect. Een footer, of welke andere output dan, ervoor of erna, is bij een redirect uberhaupt verspilde moeite. De user krijgt dat toch niet te zien.
ik vond op php.net een oplossing om na "die" toch nog html te kunnen presenteren:
Ik heb de html van de footer opgeslagen in footer.php en deze als variabele gebruikt:
<?php
function error_msg($text) {
$footer = file_get_contents('footer.php');
die($text.'<br />'.$footer);
}
error_msg('username not registered or you dont have permission to view this information');
?>
Dus in plaats van te zoeken naar hoe je nette foutafhandeling kan bouwen, ga je zoeken naar hoe je alsnog dat vermaledijde die() kan blijven gebruiken. Slim.... echt slim.
Jack, waarom helemaal gaan bouwen om een compleet verkeerde methode? waarom ga je moeilijk doen met HTML code opslaan in variabele enzo, terwijl je ook gewoon een echo kunt plaatsen?
Ik denk dat ik een en ander verkeerd begrepen heb.
Nav quote van Erwin
"En dan ga ik nu iets geks zeggen.... in het laatste script dat je post zou je het juist wel moeten gebruiken.
Na een redirect wil je namelijk dat wat er nog na komt hoe dan ook niet meer wordt uitgevoerd. Na een redirect is het dus juist wel een goede gewoonte om een exit() te plaatsen. Daarmee voorkom je dat er onverwachts toch nog een actie uitgevoerd wordt."
Ik heb dit ter harte genomen en ben daarom alsnog gaan zoeken naar een methode om na de
"die" mijn footer te kunnen plaatsen, temeer dit juist mijn laatste "loop" was en ik begreep dat "die"
command hier juist WEL geindiceerd was.
Enfin: ik heb nu een headerlocation gebruikt ipv "die" die naar pagina gaat "forbidden.html"
Is dit wel een goede foutafhandeling? Of moet ik een echo plaatsen zoals Wouter zegt?
Maak onderscheid tussen 2 situaties:
1) een databaseverbinding, query, onjuiste gebruikersinput, etc. die niet uitgevoerd kan worden. Dan bouw je nette foutafhandeling in en geen die-constructie.
2) na een header. Dan mag/kun/moet je exit gebruiken, om te voorkomen dat je script mogelijk toch verder zou gaan.
In jouw post/code van 25/12/2012 16:20:25 gebruik je 'die' terwijl je alleen maar een melding wilt tonen. Het script hoeft niet te stoppen. Waar dan wel een exit hoort, regel 24, plaats je hem niet.
In je script van 25/12/2012 22:20:22 doe je dat ook/nog fout: op regel 73 een header, zonder exit en op regel 92 gaat je script 'dood' terwijl er alleen maar een (fout)melding getoond hoeft te worden.
@ Obelix: Ik heb de aanpassingen doorgevoerd en mijn code ziet er nu als volgt uit:
<!-- PHP LOGIN GEDEELTE -->
<?php
/*
* Main.php
*
* This is an example of the main page of a website. Here users will be able to login.
* However, like on most sites the login form doesn't just have to be on the main page,
* but re-appear on subsequent pages, depending on whether the user has logged in or not.
*
* Originally written by: Jpmaster77 a.k.a. The Grandmaster of C++ (GMC)
* Last Updated by the Angry Frog: October 19, 2011
*/
include("include/session.php");
global $database;
$config = $database->getConfigs();
?>
<?php
/**
* User has already logged in, so display relavent links, including
* a link to the admin center if the user is an administrator.
*/
if($session->logged_in){
echo "<h1></h1>";
echo '<a href="index.php">My Files</a> '
.'<a href="useredit.php">Edit Account</a> ';
if($session->isAdmin()){
echo '<a href="admin/index.php">Admin Center</a> ';
}
echo '<a href="upload.php">Upload</a> '
.'<a href="process.php">Logout</a>';
/* $pagina = "users/".$session->username."/index.php";
include $pagina;*/
?>
<!-- BEGIN 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'])) {
die("<br /></br />Username not registered or you don't have permission to view this information");
}
?>
<?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)){
header('Location: forbidden.html');
}
/* 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{
die("<br /></br />Username not registered or you don't have permission to view this information");
}
/* Display requested user information - add/delete as applicable */
$req_user_info = $database->getUserInfo($req_user);
/* Username */
echo "<b>Username:</b> ".$req_user_info['username']."<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)
*/
}
?>
<!-- EINDE MY ACCOUNT INFO-->
Dit moet dan de correcte code zijn, maar dan heb ik alsnog dat footer probleem als ie "died" in de laatste loop bij /* Visitor not viewing own account */
Script consequent; niet bij de ene echo ' en de andere ", maar gebruik '.
Ik zie in je code nog steeds 'die' staan (regel 69 en 95) waar dat naar mijn idee juist niet zou moeten. En na de header, regel 85) zie ik het niet staan.
Sluit je in je script ook php af en open je daarna weer (zoals regel 49 / 56), zonder dat er veel tussendoor gebeurt?
