Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.6] Fix validation of slashes with wildcard rules #25171

Closed
wants to merge 1 commit into from
Closed

[5.6] Fix validation of slashes with wildcard rules #25171

wants to merge 1 commit into from

Conversation

staudenmeir
Copy link
Contributor

This is a modified re-submit of #25156.

#25146 (comment) describes a case where having a slash in the validation data (not in the rules) is enough to cause an exception:

$validator = Validator::make(['foo' => ['bar/' => 'baz']], ['foo.*' => 'sometimes']);
$validator->passes();

So a (malicious) user could crash the application by submitting the right data. This might not be a security risk, but certainly is an undesirable behavior.

Fixes #25146.

@jmarcher
Copy link
Contributor

Why would someone want to have slashes as array key?

I say better avoid them than merging this.

@Miguel-Serejo
Copy link

At the very least validation shouldn't throw unexpected exceptions due to user input.

While this probably wouldn't affect anything in the app, a malicious user could spam invalid requests that generate exceptions, possibly triggering alerts as they happen.

@staudenmeir
Copy link
Contributor Author

I don't see why users shouldn't be allowed to use slashes. It has been fixed for non-wildcard rules (#16309).

@jmarcher
Copy link
Contributor

Well because they just look ugly. And backslashes look like you are trying to escape something. You will need to add an extra backslash to escape the original backslash. $validator = Validator::make(['foo' => ['bar\\' => 'baz']], ['foo.*' => 'sometimes']);

@Miguel-Serejo
Copy link

@jmarcher I agree that you shouldn't use slashes in attribute names, but there's no way to prevent a user from modifying a request to add those slashes, causing your app to crash out, generating error logs, and potentially triggering a high-error-rate alert.

Sure, it's harmless to your app since it crashes during validation before any changes are made, but I don't see a downside to fixing this.

@taylorotwell
Copy link
Member

taylorotwell commented Aug 10, 2018

Don't use slashes.

@staudenmeir staudenmeir deleted the validation-slashes branch August 10, 2018 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants