PHP Security Tip #8
Withing PHP security topics, there is always more than one way to accomplish a task. Many times it's by combining tactics that we achieve the best security. We've already talked about filtering but beyond filtering we still need to be vigilant and validate input coming in from a user. This brings us to our PHP security of the day.
Always validate user input.
Take for example the following code:
<?php
$myFile = filter_var($_GET['file'], FILTER_SANITIZE_STRING);
include($myFile);
?>
Calling http://example.com/file.php?file=home.php will cause your script to include the file home.php in your current directory. However, if someone comes along and requests http://example.com/file.php?file=badcode.php you will be potentially exposing yourself to executing their code, or your code that you do not want executed in that context.
Do not depend solely on file_exists(). Just because it's a local file does not mean that it's a valid file or even that it's your file. Don't give hackers an easy easy to execute their code on your server.
To protect against this, always filter and validate:
<?php
// filter
$myFile = filter_var($_GET['file'], FILTER_SANITIZE_STRING);
// Then validate
$valid = array('home.php', 'about.php');
If (!in_array($myFile, $valid)) {
die('Leave, evil hacker');
}
include($myFile);
?>
Comments
// filter
$myFile = filter_var($_GET['file'], FILTER_SANITIZE_STRING);
// Then validate
$valid = array('home.php', 'about.php');
If (!in_array($myFile, $valid)) {
die('Leave, evil hacker');
}
...
?>
In this code
$myFile = filter_var($_GET['file'], FILTER_SANITIZE_STRING);
is not necessary
The simple answer to that question is because you should always filter input. Always. Even if the input is coming from a “trusted” source, filter the input. Once it is properly filter and there is little chance that you are not getting what you expect to get, then you validate.
Filtering and validating serve two separate purposes, both important in security.
=C=
Given the example code, I believe russain is correct: $myFile = filter_var($_GET['file'], FILTER_SANITIZE_STRING); is unnecessary. By limiting what you will accept to the values in the array, the outcome of filter_var will have no affect on the remaining code *as presented in the example.* It doesnt matter what the attacker attempts to insert into the 'file' input, if it isnt a string containing 'home.php' or 'about.php', they will be presented with the 'Leave, evil hacker' message.
Now, if the $myfile variable were to be used later in the code (ie a print/echo of 'Thank you for downloading $myfile' or something similar), then it would be CRUCIAL to include the filter. But as the example code stands now, the in_array is fulfilling both the filtering of the incoming data, and validation of the incoming data.
Given the example code, I believe russain is correct: $myFile = filter_var($_GET['file'], FILTER_SANITIZE_STRING); is unnecessary. By limiting what you will accept to the values in the array, the outcome of filter_var will have no affect on the remaining code *as presented in the example.* It doesnt matter what the attacker attempts to insert into the 'file' input, if it isnt a string containing 'home.php' or 'about.php', they will be presented with the 'Leave, evil hacker' message.
Now, if the $myfile variable were to be used later in the code (ie a print/echo of 'Thank you for downloading $myfile' or something similar), then it would be CRUCIAL to include the filter. But as the example code stands now, the in_array is fulfilling both the filtering of the incoming data, and validation of the incoming data.
Given the example code, I believe russain is correct: $myFile = filter_var($_GET['file'], FILTER_SANITIZE_STRING); is unnecessary. By limiting what you will accept to the values in the array, the outcome of filter_var will have no affect on the remaining code *as presented in the example.* It doesnt matter what the attacker attempts to insert into the 'file' input, if it isnt a string containing 'home.php' or 'about.php', they will be presented with the 'Leave, evil hacker' message.
Now, if the $myfile variable were to be used later in the code (ie a print/echo of 'Thank you for downloading $myfile' or something similar), then it would be CRUCIAL to include the filter. But as the example code stands now, the in_array is fulfilling both the filtering of the incoming data, and validation of the incoming data.
@Cal Evans
Given the example code, I believe russain is correct: $myFile = filter_var($_GET['file'], FILTER_SANITIZE_STRING); is unnecessary. By limiting what you will accept to the values in the array, the outcome of filter_var will have no affect on the remaining code *as presented in the example.* It doesnt matter what the attacker attempts to insert into the 'file' input, if it isnt a string containing 'home.php' or 'about.php', they will be presented with the 'Leave, evil hacker' message.
Now, if the $myfile variable were to be used later in the code (ie a print/echo of 'Thank you for downloading $myfile' or something similar), then it would be CRUCIAL to include the filter. But as the example code stands now, the in_array is fulfilling both the filtering of the incoming data, and validation of the incoming data.
@Cal Evans
Given the example code, I believe russain is correct: $myFile = filter_var($_GET['file'], FILTER_SANITIZE_STRING); is unnecessary. By limiting what you will accept to the values in the array, the outcome of filter_var will have no affect on the remaining code *as presented in the example.* It doesnt matter what the attacker attempts to insert into the 'file' input, if it isnt a string containing 'home.php' or 'about.php', they will be presented with the 'Leave, evil hacker' message.
Now, if the $myfile variable were to be used later in the code (ie a print/echo of 'Thank you for downloading $myfile' or something similar), then it would be CRUCIAL to include the filter. But as the example code stands now, the in_array is fulfilling both the filtering of the incoming data, and validation of the incoming data.
@Cal Evans
Given the example code, I believe russain and Piotr Borek is correct: $myFile = filter_var($_GET['file'], FILTER_SANITIZE_STRING); is unnecessary. By limiting what you will accept to the values in the array, the outcome of filter_var will have no affect on the remaining code *as presented in the example.* It doesnt matter what the attacker attempts to insert into the 'file' input, if it isnt a string containing 'home.php' or 'about.php', they will be presented with the 'Leave, evil hacker' message.
Now, if the $myfile variable were to be used later in the code (ie a print/echo of 'Thank you for downloading $myfile' or something similar), then it would be CRUCIAL to include the filter. But as the example code stands now, the in_array is fulfilling both the filtering of the incoming data, and validation of the incoming data.
@Cal Evans
Given the example code, I believe russain and Piotr Borek is correct: $myFile = filter_var($_GET['file'], FILTER_SANITIZE_STRING); is unnecessary. By limiting what you will accept to the values in the array, the outcome of filter_var will have no affect on the remaining code *as presented in the example.* It doesnt matter what the attacker attempts to insert into the 'file' input, if it isnt a string containing 'home.php' or 'about.php', they will be presented with the 'Leave, evil hacker' message.
Now, if the $myfile variable were to be used later in the code (ie a print/echo of 'Thank you for downloading $myfile' or something similar), then it would be CRUCIAL to include the filter. But as the example code stands now, the in_array is fulfilling both the filtering of the incoming data, and validation of the incoming data.