Hello and welcome to our community! Is this your first visit?
Register
Enjoy an ad free experience by logging in. Not a member yet? Register.
Page 1 of 2 12 LastLast
Results 1 to 15 of 21
  1. #1
    Regular Coder student101's Avatar
    Join Date
    Nov 2007
    Posts
    634
    Thanks
    80
    Thanked 15 Times in 15 Posts

    secure delete script?

    Is this script secure enough?
    PHP Code:
    <?php 
    // check if URL src exists... 
    if (isset($_GET['src'])){
    // setup folder paths to $vars... 
    $path="path/to/folder/".$_GET['src'];
    $path2="path2/to/folder/".$_GET['src'];
     
    // check if the file exists
    if (file_exists($path)) {
     
    // check if the file is deleted
    if(@unlink($path)) {
     
    // goto the page you need to go to...
    $deleteGoTo "page_to_goto.php";
    header(sprintf("Location: %s"$deleteGoTo));
    }
     
    }else{
     
    // check if the file is deleted...
    if(@unlink($path2)) {
     
    // goto the page you need to go to...
    $deleteGoTo "page_to_goto.php";
    header(sprintf("Location: %s"$deleteGoTo));
    }
    }
     
    }else{ 
    // goto the page if URL src exists...
    $deleteGoTo "page_to_goto.php";
    header(sprintf("Location: %s"$deleteGoTo));
    }
    ?>
    Thanks for your support!
    Update MySQL with checkboxes | Tell A Friend | Delete MySQL with checkboxes

    Give thanks & resolve when done :thumbsup:

  • #2
    Senior Coder
    Join Date
    Jul 2009
    Location
    South Yorkshire, England
    Posts
    2,318
    Thanks
    6
    Thanked 304 Times in 303 Posts
    What would happen if someone supplied the following, (or suchlike), as the src value in $_GET['src']?

    ../../../../file

  • #3
    Super Moderator Inigoesdr's Avatar
    Join Date
    Mar 2007
    Location
    Florida, USA
    Posts
    3,647
    Thanks
    2
    Thanked 406 Times in 398 Posts
    Quote Originally Posted by MattF View Post
    What would happen if someone supplied the following, (or suchlike), as the src value in $_GET['src']?

    ../../../../file
    The script would attempt to delete a file 4 levels above the directory specified in $path. So student101, to answer your question in a word: No.

  • #4
    Regular Coder student101's Avatar
    Join Date
    Nov 2007
    Posts
    634
    Thanks
    80
    Thanked 15 Times in 15 Posts
    Ok so what does ../../../../file do or is it metophorical?

    PHP Code:
    // check if URL src exists... 
    if (isset($_GET['src'])){
     
     
    // escapes special characters (PHP 4 >= 4.3.0, PHP 5)
    $thesrc mysql_real_escape_string($_GET['src']);
     
     
    // setup folder paths to $vars... 
    $path="path/to/folder/".$thesrc;
    $path2="path2/to/folder/".$thesrc;
     
    // check if the file exists
    if (file_exists($path)) {
     
    // check if the file is deleted
    if(@unlink($path)) {
     
    // goto the page you need to go to...
    $deleteGoTo "page_to_goto.php";
    header(sprintf("Location: %s"$deleteGoTo));
    }
     
    }else{
     
    // check if the file is deleted...
    if(@unlink($path2)) {
     
    // goto the page you need to go to...
    $deleteGoTo "page_to_goto.php";
    header(sprintf("Location: %s"$deleteGoTo));
    }
    }
     
    }else{ 
    // goto the page if URL src exists...
    $deleteGoTo "page_to_goto.php";
    header(sprintf("Location: %s"$deleteGoTo));

    On another note; would this be of worth?
    PHP Code:
    // convert numeric url request to $var
    $id = (int)$_GET['id'];
     
    // check if $var is valid and an integer
    if (isset ($id) && $id != "" && (is_int($id)) ){
     
    // ... do things here

    Last edited by student101; 02-21-2010 at 08:26 PM.
    Thanks for your support!
    Update MySQL with checkboxes | Tell A Friend | Delete MySQL with checkboxes

    Give thanks & resolve when done :thumbsup:

  • #5
    Senior Coder
    Join Date
    Jul 2009
    Location
    South Yorkshire, England
    Posts
    2,318
    Thanks
    6
    Thanked 304 Times in 303 Posts
    ../ traverses up the directory tree. You're going one dir up for every ../.

    You should allow alphanumeric characters, underscores, a single period and hyphens at most in your src var. There's no reason for any other characters to be there. Sanitise the path.

  • #6
    Regular Coder student101's Avatar
    Join Date
    Nov 2007
    Posts
    634
    Thanks
    80
    Thanked 15 Times in 15 Posts
    Good point, will do!
    PHP Code:
    $pathmysql_real_escape_string("path/to/folder/".$thesrc);
    $path2mysql_real_escape_string("path2/to/folder/".$thesrc); 
    Should I do the check first then assign $vars or vice versa?
    Code:
     
    $id = (int)$_GET['id'];
    if (isset ($id) && $id != "" && (is_int($id)) ){
     
    OR 
     
    if (isset ($_GET['id'];) && $_GET['id'];!= "" && is_int($_GET['id'];) ){
    $id = (int)$_GET['id'];
    



    Thanks for your support!
    Update MySQL with checkboxes | Tell A Friend | Delete MySQL with checkboxes

    Give thanks & resolve when done :thumbsup:

  • #7
    Senior Coder
    Join Date
    Jul 2009
    Location
    South Yorkshire, England
    Posts
    2,318
    Thanks
    6
    Thanked 304 Times in 303 Posts
    Quote Originally Posted by student101 View Post
    Good point, will do!
    PHP Code:
    $pathmysql_real_escape_string("path/to/folder/".$thesrc);
    $path2mysql_real_escape_string("path2/to/folder/".$thesrc); 
    That will make bugger all difference to the particular scenario.

    For the other question:

    Code:
    $id = ((isset($_GET['id'])) ? intval($_GET['id']) : 0);
    
    if ($id > 0)
    {
        [code here]
    }
    Last edited by MattF; 02-21-2010 at 08:51 PM.

  • Users who have thanked MattF for this post:

    student101 (02-21-2010)

  • #8
    Regular Coder student101's Avatar
    Join Date
    Nov 2007
    Posts
    634
    Thanks
    80
    Thanked 15 Times in 15 Posts
    Quote Originally Posted by MattF View Post
    That will make bugger all difference to the particular scenario.
    I take it something is incorrect?
    Thanks for your support!
    Update MySQL with checkboxes | Tell A Friend | Delete MySQL with checkboxes

    Give thanks & resolve when done :thumbsup:

  • #9
    Senior Coder
    Join Date
    Jul 2009
    Location
    South Yorkshire, England
    Posts
    2,318
    Thanks
    6
    Thanked 304 Times in 303 Posts
    Quote Originally Posted by student101 View Post
    I take it something is incorrect?
    You're escaping the input for insertion into the DB. You aren't doing any sanitisation of the path whatsoever through the use of *escape_string().

  • #10
    Regular Coder student101's Avatar
    Join Date
    Nov 2007
    Posts
    634
    Thanks
    80
    Thanked 15 Times in 15 Posts
    Quote Originally Posted by MattF View Post
    Sanitise the path.
    What other path is there?
    "path/to/file".$thesrc makes up the path..
    Thanks for your support!
    Update MySQL with checkboxes | Tell A Friend | Delete MySQL with checkboxes

    Give thanks & resolve when done :thumbsup:

  • #11
    Senior Coder
    Join Date
    Jul 2009
    Location
    South Yorkshire, England
    Posts
    2,318
    Thanks
    6
    Thanked 304 Times in 303 Posts
    Quote Originally Posted by student101 View Post
    What other path is there?
    "path/to/file".$thesrc makes up the path..
    Code:
    // escapes special characters (PHP 4 >= 4.3.0, PHP 5)
    $thesrc = mysql_real_escape_string($_GET['src']);
     
     
    // setup folder paths to $vars... 
    $path="path/to/folder/".$thesrc;
    $path2="path2/to/folder/".$thesrc;
    If the value of $GET['src'] is something along the lines of ../../../../[the name of any file some cretin wishes to delete] or similar, you are effectively allowing your users to delete, (provided your httpd user has permissions), any file anywhere on your system, if they can find a valid path. That example value above would mean that the user is now accessing files outside of the httpd root directory.

  • #12
    Senior Coder kbluhm's Avatar
    Join Date
    Apr 2007
    Location
    Philadelphia, PA, USA
    Posts
    1,509
    Thanks
    3
    Thanked 258 Times in 254 Posts
    The first thing you should do, before even trying to delete anything, is check if some type of admin access has been granted. You still want to sanitize the deleted file paths afterward, but it is much less likely that someone you have trusted with admin access will be toying with the script looking for exploits.

  • #13
    Regular Coder student101's Avatar
    Join Date
    Nov 2007
    Posts
    634
    Thanks
    80
    Thanked 15 Times in 15 Posts
    So what would you recommend I do to secure the URL request?

    Edit:
    admin access has been granted?
    Yes suPHP is enabled on the server.
    Thanks for your support!
    Update MySQL with checkboxes | Tell A Friend | Delete MySQL with checkboxes

    Give thanks & resolve when done :thumbsup:

  • #14
    Senior Coder kbluhm's Avatar
    Join Date
    Apr 2007
    Location
    Philadelphia, PA, USA
    Posts
    1,509
    Thanks
    3
    Thanked 258 Times in 254 Posts
    Well, in what context is this URL being used? Is it linked directly from an open page for anyone to click? Is it already behind a secured page? Is it being included with an already secured file?

  • Users who have thanked kbluhm for this post:

    student101 (02-21-2010)

  • #15
    Senior Coder
    Join Date
    Jul 2009
    Location
    South Yorkshire, England
    Posts
    2,318
    Thanks
    6
    Thanked 304 Times in 303 Posts
    Quote Originally Posted by kbluhm View Post
    The first thing you should do, before even trying to delete anything, is check if some type of admin access has been granted. You still want to secure the deleted files path afterward, but it is less likely that someone you have trusted with admin access will be toying with the script looking for exploits.
    I'd say the opposite, personally. Scripts should be secure by default, before any type of permissions system is taken into account. Sanitisation and validation should be the first port of call, not the second.


  •  
    Page 1 of 2 12 LastLast

    Posting Permissions

    • You may not post new threads
    • You may not post replies
    • You may not post attachments
    • You may not edit your posts
    •