-1

i have this url i want to pass with parameters http://localhost/my_site/page.php?p=some-string.

now, i have a function that i use to try to make this url safe. I want to make it so that there can only contain letters, numbers and the - character.

function clear($url) {
    $url = trim($url);
    $url = preg_replace("/[^a-zA-Z0-9\-]/", "", $url);

    return $url;
}
$p = $_GET['p'];
$p = clear($p);

also in htaccess, i have this rule

RewriteCond     %{REQUEST_FILENAME} !-f
RewriteRule     ^page/([A-Za-z0-9\-]+)/?$    page.php?p=$1   [L]

is this enough to make the url safe?

Richard
  • 325
  • 7
  • 23
eskimopest
  • 401
  • 3
  • 18
  • You don't need to go to that extent for a GET variable – Richard Apr 21 '18 at 12:32
  • "Safe" depends on the usage and context, which this question did not go into detail about. – mario Apr 21 '18 at 12:48
  • 3
    You want to make url safe - against *what*? What you're doing has nothing to do with security or safety. – N.B. Apr 21 '18 at 12:57
  • @N.B. i have a xml file and need the parameter to retrieve data from it. just want to make sure people wont mess with the url. the htaccess is only to make url "nicer". hope this helps – eskimopest Apr 21 '18 at 14:03
  • This does indeed have a lot to do with security. It is an effective and quite strong input validation for url parameters. While this alone won't make anything secure, it is a good building block, and is useful against injection-type attacks. So I think it's good to have this filter. – Gabor Lengyel Apr 21 '18 at 14:43
  • When you sanitized it on your clear function, you can return false when the sanitized string does not match the input. In that case don't even run the query and show the proper message to the front end user. You can add more to the clear function like checking strlen(), is_string(), ... – ICE Apr 21 '18 at 16:02

1 Answers1

-1

.htaccess makes for a poor security layer, especially if someone deploys on nginx instead of Apache. There's nothing preventing someone from visiting /page.php?p=' OR 1=1;-- is there?

Your subsequent clear() function might be adequate to strip any unwanted characters, but whether or not this is "safe" depends on what the rest of your code does with $p after you "clear" it.

There are two variants of this question that you might have also meant to ask:

  1. How to make sure user input doesn't lead to a security vulnerability?
    • This is domain-specific, you won't find a one-size-fits-all general solution.
    • If you're worried about SQL injection, just use prepared statements and don't worry about what gets passed to the URL.
    • If you're worried about XSS, LFI, RFI, etc. then there are other domain-specific security controls you can implement too.
  2. How to make sure user input is valid.
    • Ionizer may be helpful here. It's an open source input filtration/validation library meant to process structured input (multi-dimensional arrays i.e. $_GET and $_POST).

For example, you could do something like this:

<?php
use ParagonIE\Ionizer\GeneralFilterContainer;
use ParagonIE\Ionizer\Filter\{
    StringFilter,
    WhiteList
};

// Define properties to filter:
$ic = new GeneralFilterContainer();
$ic->addFilter(
        'page',
        (new StringFilter())
            ->setPattern('^[A-Za-z0-9_\-]{3,24}$')
    );

// Invoke the filter container on the array to get the filtered result:
try {
    // $get passed all of our filters.
    $get = $ic($_GET);
} catch (\TypeError $ex) {
    // Invalid data provided.
}

Now you just have to define your filters and pass the input array ($_GET, $_POST, $_REQUEST, json_decode(file_get_contents('php://input'), true), etc.) and you can enforce whatever pattern you want on input strings, or guarantee that certain variables are numeric, etc.

Scott Arciszewski
  • 33,610
  • 16
  • 89
  • 206