dishadcruze Posted July 14, 2017 Share Posted July 14, 2017 i am checking whether the logged in user has access to the page he is trying to view through a script. I have a table pages where i store page ids with links like this and in a table access_level i store user's access now if a user clicks on delete button echo "<td><a onClick=\"javascript: return confirm('Please confirm deletion');\" href='delete_lead.php?id=".$row['LID']."'><div class='label label-danger'><i class='fa fa-trash' title='Delete'></i></div></a></td> "; he should not be able to delete but he should be redirected to no_access.php. This is working file for all other files except delete file. my delete file is like this include('../access.php'); $id = $_GET['id']; $sql = mysqli_query($con, "delete from leads where id=".$id."") or die (mysqli_error($con)); if($sql) { header("location:leads_view.php"); } My access checking file is like this <?php ob_start(); include("connect.php"); include("admin_auth.php"); $q1 = basename($_SERVER['REQUEST_URI'], '?' . $_SERVER['QUERY_STRING']); $q2 = $_SERVER['REQUEST_URI']; $var1 = "/".$q1; $qa_path=explode('/', $q2); $right_path = $qa_path[2].$var1; $parsedUrl = parse_url($q2); $curdir = dirname($_SERVER['REQUEST_URI'])."/"; $l1 = "select teams from team_members WHERE user_id=".$_SESSION['user_id']." "; $l2 = mysqli_query($con, $l1) or die(mysqli_error($con)); $cnt = mysqli_num_rows($l2); if($cnt>0) { $l3 = mysqli_fetch_array($l2); $teams = $l3['teams']; $m1 = "select pages.page_id, pages.code, pages.page, pages.href, access_level.aid, access_level.page_id, access_level.user_id FROM pages INNER JOIN access_level ON pages.page_id=access_level.page_id WHERE access_level.user_id=".$_SESSION['user_id']." OR access_level.user_id IN('".$teams."')"; $m2 = mysqli_query($con, $m1) or die (mysqli_error($con)); while($nk = mysqli_fetch_array($m2)) { $href[] = ($nk['href']); } if(in_array($right_path, $href)) { echo "<script type='text/javascript'> document.location = ".BASE_URL."/".$right_path."</script>"; } else { echo "<script type='text/javascript'> document.location = '../no_access.php' </script>"; } } else if($cnt==0) { $m1 = "select pages.page_id, pages.code, pages.page, pages.href, access_level.aid, access_level.page_id, access_level.user_id FROM pages INNER JOIN access_level ON pages.page_id=access_level.page_id WHERE access_level.user_id=".$_SESSION['user_id'].""; $m2 = mysqli_query($con, $m1) or die (mysqli_error($con)); while($nk = mysqli_fetch_array($m2)) { $href[] = ($nk['href']); } if(in_array($right_path, $href)) { echo "<script type='text/javascript'> document.location = ".BASE_URL."/".$right_path."</script>"; } else { echo "<script type='text/javascript'> document.location = '../no_access.php' </script>"; } } record is getting deleted , not getting why Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted July 14, 2017 Share Posted July 14, 2017 (edited) First the standard decontamination procedure -- which is going to take a while in your case: Your code has SQL injection vulnerabilities all over the place. Not only can this be used to compromise your database or even the entire server. It also leads to syntax errors with perfectly valid input. Learn to use prepared statements. Your error handling is messed up. You dump your errors straight on the website, which is really helpful for attackers and very irritating for legitimate users. Learn how to enable exceptions and then let PHP handle them. mysqli is generally a poor choice. It's a cumbersome low-level interface for people who read the manual, and we all know this doesn't work for PHP programmers. If you can, switch to PDO. It's far more programmer-friendly and supports many different database systems, not just MySQL. You have no protection whatsoever against cross-site request forgery attacks. I could send a request through your browser right now simply by including an image on this forum, and your application would happily accept it: <img src="https://yoursite.com/delete_lead.php?id=123" alt=""> Using a GET request to change data violates the HTTP protocol and comes with all kinds of problems. There's a reason why it's called “GET”: It's for getting data. If you want to change data, you need the POST method (or DELETE, but this is poorly supported). You seem to fundamentally misunderstand which parts of the code are executed when and by whom. JavaScript code runs in the browser, which means that a) the user is free to ignore it and b) it runs after the PHP code. In other words, your “access control” consists of letting the user do whatever they want, and then afterwards you ask them to please stop the action which has already happened and go to a different page. This obviously doesn't make much sense. Any access control must be enforced server-side by stopping the script. Given those fundamental problems, it might be a good idea to start from scratch. I understand this is frustrating, but at least you'll learn from it and (hopefully) end up with an application that actually does what it should do. A working access control script will have to look like this: <?php // if the user doesn't have access ... if (!has_access_to_this_feature()) { // ... set the right error code (e. g. 403 Forbidden) http_response_code(HTTP_CODE_FORBIDDEN); // ... tell them (don't just do a silent redirect); feel free to insert a pretty HTML page here echo 'You do not have access to this page.'; // ... then stop the script; this is the most important part exit; } // at this point, the access check was successful; now do your other checks (CSRF, invalid data etc.) Edited July 14, 2017 by Jacques1 1 Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.