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

[9.x] Improve '$user' variable handling in PostgreSQL search_path #35567

Merged
merged 1 commit into from
Dec 11, 2020

Conversation

cbj4074
Copy link
Contributor

@cbj4074 cbj4074 commented Dec 10, 2020

Upon closer scrutiny of PostgreSQL CVE-2018-1058 , which I first noted in laravel/ideas#918 (comment) , a very slight tweak should be made to how the framework parses the search_path, as configured on the database connection.

To be clear, the current implementation is not in itself vulnerable to this CVE. However, the framework does not yet fully-support the best-practice configuration described in the PostgreSQL documentation, in relation to aforementioned CVE.

It bears explaining that, in addition to literal schema names, the search_path accepts a special variable, $user, that PostgreSQL resolves to the effective username. So, if the search_path is defined as "$user", public, PostgreSQL resolves that to foouser, public, internally, when executing queries. All of this is completely transparent to Laravel, as of #35463 (which, among other things, adds support for $user in the search_path configuration variable), with one exception, which is when querying PostgreSQL's information_schema to determine, e.g., whether a given table exists or which columns exist on a given table.

This PR modifies how the search_path is parsed when building queries to be executed against the information_schema to account for the possibility that the first schema listed in the search_path is the special $user variable.

I should underscore that including this variable in the search_path is not some exotic use-case that nobody would ever employ; the default value in a PostgreSQL installation is "$user", public. Granted, Laravel overrides this default with the search_path value specified in the connection's configuration, but it's certainly conceivable that an end-user would want to configure Laravel to use the best-practice value, which is actually just $user.

In fact, I think consideration should be given to changing Laravel's default from public to $user (in laravel/laravel), because the latter is safer, for the reason described in the PostgreSQL documentation (emphasis mine):

Constrain ordinary users to user-private schemas. To implement this, issue REVOKE CREATE ON SCHEMA public FROM PUBLIC, and create a schema for each user with the same name as that user. Recall that the default search path starts with $user, which resolves to the user name. Therefore, if each user has a separate schema, they access their own schemas by default. After adopting this pattern in a database where untrusted users had already logged in, consider auditing the public schema for objects named like objects in schema pg_catalog. This pattern is a secure schema usage pattern unless an untrusted user is the database owner or holds the CREATEROLE privilege, in which case no secure schema usage pattern exists.

With the changes in this PR, if $user is the first schema in the search_path, the PostgresBuilder will resolve that schema name to the username defined on the database connection whenever appropriate, e.g., in the hasTable() and getColumnListing() methods.

With this change in place, Laravel's search_path handling should now be uniform, consistent, and capable of supporting the best-practices described in the PostgreSQL documentation; specifically, configuring the search_path to begin with $user.

…rch_path

Given that the default search_path value in a PostgreSQL installation is '"$user", public', or perhaps just '"$user"', as may be the case if hardened against CVE-2018-1058, it is preferable to account for the possibility that an end-user may wish to configure a PostgreSQL database connection in Laravel to mimic said default.

Now, if '$user' is the first schema in the search_path, the PostgresBuilder will resolve that schema name to the username defined on the database connection whenever appropriate, e.g., in the hasTable() and getColumnListing() methods.
@cbj4074 cbj4074 marked this pull request as ready for review December 10, 2020 20:53
@cbj4074
Copy link
Contributor Author

cbj4074 commented Dec 10, 2020

@taylorotwell This is the last one, I promise! 😄

I'm happy to update the Upgrade Guide document and submit a PR for it, too, if that's helpful.

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