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

[8.x] Wrap column in aggregate function #35004

Merged
merged 1 commit into from Oct 28, 2020
Merged

Conversation

sebdesign
Copy link
Contributor

In general, columns in raw expressions need to be wrapped for safety.

This PR wraps the $column parameter in the aggregate $function, because it's easy to pass arbitrary SQL as an argument to the $column parameter.

@GrahamCampbell GrahamCampbell changed the title Wrap column in aggregate function [8.x] Wrap column in aggregate function Oct 27, 2020
@taylorotwell
Copy link
Member

Just want to note that wrapping doesn't prevent SQL injection. You can't bind column names in PDO as bound parameters so they have to be concatenated into the query as plain strings. The wrapping just prevents the DB from erroring if the column name is a reserved word used by the DB.

@taylorotwell taylorotwell merged commit 50fd3c6 into laravel:8.x Oct 28, 2020
@sebdesign
Copy link
Contributor Author

Thank you for the clarification, I assumed that the wrapping would provide some basic security apart from preventing SQL errors.

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.

None yet

2 participants