Hey,
I have a wire:click within my app that looks like this:
wire:click="profileClick('2270ef7f-8fc1-11ea-92e3-0050563cf67f')"However, if I inspect that element, and change it to:
wire:click="profileClick('hello')"It is passed to the Livewire controller without any flag raised.
I can see this being a major cyber-security concern around actions which have more sensitive functionality.
What are people’s thoughts around this?
You need to check what you receive on profileClick function, and that is a valid value. It is a security issue that you may have in many places, not only livewire.
Its no different to any form submission. NEVER trust user input. Livewire components are still rendered client side so are not to be trusted.
you can use Crypt facade in Laravel with build in encrypt and decrypt methods and ending up the method in your controller with try and catch like this:
in your view:
...
wire:click="profileClick(Crypt::encrypt(''2270ef7f-8fc1-11ea-92e3-0050563cf67f'))"
...
in your controller
...
$file = request('your request name here');
try {
$key = $file;
$file = Crypt::decrypt($key);
} catch (DecryptException $e) {
return abort(422);
}
...
to insure the incoming data is correct and encrypted.
as @Snapey said: never trust user inputs.
I like @skywalker’s answer in that I don’t know how long, if ever, I would have come up with that. The problem, in my opinion, is that it’s a bandaid fix for a bigger problem you have. If you are hardcoding a parameter into a method call in the page wire:click="method($param)" you have a fundamental design flaw that is opening yourself up where not needed. You should only be using wire:click="method" and handling all of your parameters on the back end so there is no way for the user to inject a parameter.
I went into more detail of why I believe this in my rant here: Specific Loading target
Thats often not possible when, for instance, you are clicking on rows of a table. The Livewire component cannot know what row you clicked on unless you tell it. You could maintain an array and use an index on both sides rather than sending actual model keys?
@Snapey I respectfully disagree. It’s only not possible if you are using a design pattern that in my opinion is flawed and I think using a table is the perfect example of this. Let’s use a simple example with all of your users that lists their name, email, a button to grant admin status, and a button to delete. Let’s also ignore the practicality that it is an admin function you wouldn’t have to be as security conscious about user input, but it will make due for the purposes of an example here.
Single God Component design:
You have one component that is responsible for displaying the table, filtering/searching/sorting, pagination, taking a user id to change role, taking a user id to delete.
Downsides:
Upsides:
Parent Component for table structure, Child Components for each user model (each table row):
You have one component that is responsible for displaying the table, filtering/searching/sorting, pagination. You have multiple children components that each own a model, responsible for changing the role and deleting.
Downsides:
Upsides:
wire:clicks are only a method name, no parameters hard coded into the html of the page, or accepted by your methods on the component backend, so there’s nothing to be tampered with. If you know where to look (and aren’t hiding it), you can try, but livewire will error out.Comparing the two, I just see the first way is a bad design pattern. The second is more efficient with database queries and request sizes, you aren’t introducing an additional avoidable security opening that you now to also have to protect against - creating more work and code to maintain, and it adheres to the single responsibility principle a whole lot closer that the first way does. Unless there’s something I’m just blatantly missing, the pros far outweigh the cons of going with the second pattern.
I have yet to come across any reason to pass a method a parameter within the html. Anything that takes a user input can be modeled to a property, and if you cant accept input that way, you shouldn’t be accepting it as a method parameter on a livewire public method in the first place.
And the case where you are not simply clicking on the row to change its status ‘in line’ but are instead redirecting to the edit route for that resource?
I don’t think I understand your question. The way I’m reading it is you just have a link to a route, nothing to do with livewire methods or hard coding parameters into the html. Or are you talking exposing database id’s in the route? If so, I set up slugs for those.