Top 8 bad security practices in Laravel you should be aware of
The true security comes within the code practice itself, we can’t determine that a framework is secure or not, Laravel is as secure as it can be and optimized as it can be, it really depends on the developer.
Here I will list top 8 bad security practices with tips for you to avoid them.
- SQL Injection via column names and raw queries.
- SQL Injection via validation rules.
- Validation rules injection.
- Using GET for non-GET routes.
- Using $request->all().
- Forgetting the Debug Mode on in production.
- XSS using unescaped data statement.
- XSS using href Attribute.
SQL Injection via column names :
A very common practice passing user-controlled columns to Query Builder thinking that Laravel validates it via PDO parameter binding, but as Laravel Documentation mentions :
PDO does not support binding column names. Therefore, you should never allow user input to dictate the column names referenced by your queries, including “order by” columns.
To make it clear let’s say that we have the following query :
Product::where('brand_id',$request->brand_id)
->orderBy($request->orderBy)
->get();so if the user entered something like price->”%27))%23injection the query will be translated as following :
select * from `products` where brand_id = ? order by json_unquote(
json_extract(`price`, '$.""')
)#injection"')) ascWhat should you do : as Laravel Documentation mentions :
if must allow the user to select certain columns to query against, always validate the column names against a white-list allowed columns
It’s fair to also mention using any raw query function has the same issue if you were using it depending on user input and with no binding.
So don’t forget to use Parameter Binding
SQL Injection via validation rules
As mentioned before, you should be very aware of using user inputs in your query, one of the practices that developer doesn’t pay attention to is building validation rules base on provided data from user, let’s take a closer look.
This practice is common when validating an update request and provide the id from the request itself, Here is an example
$id = $request->id;
$rules = [
'email'=>'email|required|unique:users,email,'.$id
];
$validator = Validator::make($request->all(),$rules);There we can see it is security vulnerability that allows hacker to inject SQL statement.
What should you do: simply, don’t build validation rules based on your user entry, if you are forced to, just validate the values manually and then build your rule.
Validation Rules Injection :
1- Make the rule optional:
as the example before mentioned
$id = $request->id;
$rules = [
'email'=>'email|required|unique:users,email,'.$id
];
$validator = Validator::make($request->all(),$rules);the user passed the following :
10|sometimeswhich will lead for the rule to be optional, depending on the businesses logic of the application this rule injection could make a lot of damage.
2- DDOS regex validation :
Let’s pretend that the user send this regex :
1|regex:(*a){10000}The previous regex would consume a lot of resources and multiple requests with the same payload would overflow the CPU.
What should you do: As Mentioned Before : simply, don’t build validation rules based on your user entry, if you are forced to, just validate the values manually and then build your rule.
Using GET for non-GET routes
As mentioned in Laravel Documentation :
Anytime you define a “POST”, “PUT”, “PATCH”, or “DELETE” HTML form in your application, you should include a hidden CSRF
_tokenfield in the form so that the CSRF protection middleware can validate the request
By default, Laravel would fail any request from the mentioned actions with no _token field.
So why did we mentioned this?
Through out my career I’ve seen many developers define a GET route for non-GET Actions, like for example this route
Route::get('product/{id}/delete','ProductController@delete);For previous route, if it was called it will do the delete action cause laravel won’t check for the _token field and the action will be executed.
What should you do :
be careful when defining your route, use every method for its appropriate action or event.
Using $request->all() to create model
Now here comes the juicy part,
I have seen many developers deploy the following code :
\App\Models\User.php
<?php
namespace App\Models;
class User extends Authenticatable
{
protected $fillable = [
'usernamename',
'email',
'password',
'role'
];
}UserRequest.php
<?php
namespace App\Http\Requests;
class UserRequest extends FormRequest
{
public function rules()
{
return [
'username'=>'required',
'email'=>'email|required|unique:users,email'
'password'=>'required|min:8'
];
}
}UserController.php
<?php
namespace App\Http\Controllers;
class UserController extends Controller
{
public function store(UserRequest $request)
{
User::create($request->all());
return response()->json();
}
}So many developers think that by using FormRequest.php validation the $request->all() would only return the validated data, WRONG
for the previous example if the user send the payload as :
{
"username":"test",
"email":"email@email.com",
"password":"Pssw0rd",
"role":"admin"
} The user Test would be an admin and boom.
That is a simple example and So go on with your imagination.
What should you do :
instead of using $request->all()
you can use $request->validated() or $request->only().
Forgetting the Debug Mode on, in production
So we done every thing we could to make the application secure, One last thing is DON’T FORGET THE APP_DBUG=TRUE in .env file.
Leaving the debug mode on will reveal some sensitive parts of your code to hackers or even some configuration data and 3rd party credentials.
for example I created a function to throw exception as the following
The exception page revealed my User model file content to public:
What should you do:
simply, set APP_DEBUG in .env file to false.
if you want to keep track of your exceptions log just add the following code to your App\Exceptions\Handler.php file:
protected function LogException(\Exception $ex){
Log::error("Message : ".$ex->getMessage());
Log::error("Line : ".$ex->getLine());
Log::error("File : ".$ex->getFile());
}
public function register(){
$this->reportable(function (Throwable $e) {
$this->LogException($exception);
});
}XSS using unescaped data statement
By default, Blade {{ }} statements are automatically sent through PHP's htmlspecialchars function to prevent XSS attacks. If you do not want your data to be escaped, you may use the following syntax:{!! $data !!}
it’s a big XSS vulnerability to unescape user data in your blade view.
to make the difference let’s see the following example :
$data = '<b>Bold</b>';
-----------------------
<body>
this is escaped {{$data}}
this is unescaped {!! $data !!}
</body>the page will be as following
for example if $data was a user input like :
$data = '<script>alert("XSS attack");</script>';the result would be :
What should you do :
be careful and don’t use unescaped statement {!! !!} with data provided by user, Never trust user input
XSS using href Attribute
not only {!! !!} allows the execution of malicious code on the client side but also it can be done using the href attribute in tag <a>.
so for example, your application allow users to share links and you created an <a> tag to show the links and redirect users to links, if one of the users stored a malicious code as link it could lead to unhappy ending :
$data = 'javascript:alert("XSS attack");';
--------------------------------------------------
.blade file : <a href="{{$data}}">Click here to see more link</a>
So when the user clicks on it :
What should you do:
if the users provides links to put on your blade files and be shared dynamically, you can simply Validate http/https schema before storing the link.
If I forget some, please remind me in the comments.
Thanks for reading.
