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

Bug: Field permissions not enforced on WHERE statemets #2161

Open
2 tasks done
selmeci opened this issue Jun 20, 2023 · 2 comments
Open
2 tasks done

Bug: Field permissions not enforced on WHERE statemets #2161

selmeci opened this issue Jun 20, 2023 · 2 comments
Assignees
Labels
bug Something isn't working topic:security This is related to security topic:surrealql This is related to the SurrealQL query language

Comments

@selmeci
Copy link

selmeci commented Jun 20, 2023

Describe the bug

There is a bug in the permission application in the WHERE clause of the SELECT statement. The problem manifests when a user who doesn't have the necessary privileges tries to select records based on a field that they don't have read access to. The issue is that the SELECT statement should not execute successfully if the WHERE clause refers to a field the user doesn't have permission to read. The current behavior inadvertently leaks information to the user about the underlying data, which is a violation of the principle of least privilege.

Steps to reproduce

  1. Define a table with permissions. In the example provided, the table is called car and the specific field with permissions is color. Permissions are defined for select, create, update, and delete operations for the field.

  2. Create a record in the car table with the color field set to green.

  3. Authenticate as a user without the 'Car/color/Read' privilege.

  4. Execute a SELECT statement with a WHERE clause that includes the color field, such as SELECT * FROM car WHERE color = 'green';.

  5. Observe the returned response. Although the color field is correctly omitted from the response due to the user's lack of read permission, the fact that a record is returned at all indicates that there is a car with a green color, leaking information about the underlying data.

Table:

db
    .query("DEFINE TABLE car;")
    .query(
        r#"
        DEFINE FIELD color ON TABLE car TYPE string 
            ASSERT $value != NONE AND string::len($value) > 2
            PERMISSIONS
                FOR select
                    WHERE $auth.privileges CONTAINS 'Car/color/Read'
                FOR create
                    WHERE $auth.privileges CONTAINS 'Car/color/Create'
                FOR update
                    WHERE $auth.privileges CONTAINS 'Car/color/Update'
                FOR delete
                    WHERE $auth.privileges CONTAINS 'Car/color/Delete'
        ;"#,
    )
    .await?
    .check()?;

Query:

let response = user_a_db
    .query(
        r#"
        SELECT * FROM car WHERE color = 'green';
        "#,
    )
    .await?;
dbg!(response);

Response:

Response(
    {
        0: Ok(
            [
                Object(
                    Object(
                        {
                            "id": Thing(
                                Thing {
                                    tb: "car",
                                    id: String(
                                        "l4zdqfi7oxrm5dl1h2d4",
                                    ),
                                },
                            ),
                        },
                    ),
                ),
            ],
        ),
    },
)

Even though the user does not have read permission for the color field, the SELECT statement with a WHERE clause referring to that field is executed successfully, and a record is returned. The color field is correctly omitted from the returned record, but the mere presence of the record indicates that there is a car with a green color, effectively leaking information about the underlying data.

Expected behaviour

If a user does not have read permission for a field, any SELECT statement with a WHERE clause referring to that field should result in an error, or at least not return any records. This would maintain the principle of least privilege and avoid leaking information about the underlying data.

SurrealDB version

1.0.0-beta.9+20230402.5eafebd for linux on x86_64

Contact Details

selmeci.roman@gmail.com

Is there an existing issue for this?

  • I have searched the existing issues

Code of Conduct

  • I agree to follow this project's Code of Conduct
@selmeci selmeci added bug Something isn't working triage This issue is new labels Jun 20, 2023
@tobiemh tobiemh added bug Something isn't working and removed bug Something isn't working triage This issue is new labels Jun 20, 2023
@sgirones
Copy link
Member

I did some work around authotisation in #2176, I will see if this bug is still present and try to fix it there.

@sgirones sgirones self-assigned this Jun 23, 2023
@finnbear
Copy link
Contributor

IMO a permission error would leak information that the field exists, so treating the field as NONE and letting the WHERE predicate evaluate as-is would be ideal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working topic:security This is related to security topic:surrealql This is related to the SurrealQL query language
Projects
None yet
Development

No branches or pull requests

6 participants