8000 Implemented static analysis for client side permissions by scudette · Pull Request #4246 · Velocidex/velociraptor · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Implemented static analysis for client side permissions #4246

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

Merged
merged 7 commits into from
May 20, 2025
Merged

Conversation

scudette
Copy link
Contributor

Client side artifacts run on the endpoint without ACL enforcement. This is usually what we want but sometimes the extra permissions can give the user extra permissions on the end point.

The artifact writer can specify required permissions which will be enforced by the server prior to collecting the artifact.

However, sometimes the artifact can use plugins with high priviledge safely - in that case we do not want to restrict the users that may collect it.

For example say the artifact collects autoruns by shelling to the autoruns.exe tool. Even though it is using the execve() plugin this call is safe because the args are fixed and can not be influenced by the user. However if the artifact passed user input into the execve() plugin, the user requires the EXECVE permission.

Previously, the artifact could declare EXECVE as a required permission, for artifact uses where user input was directly allowed in execve() calls. This enforces additional checks on the launching user to ensure they have the EXECVE permission.

This PR modifies the artifact verifier to track plugin permissions used in the artifact. This allows us to see if the artifact inadvertantly gives the user permissions they do not have.

This PR introduces another field to the artifact definition called implied_permissions where the artifact writer can declare permissions which the artifact will give but the user does not require those. This helps the verifier identify additional permissions that are accidentally given to users on the client.

This PR adds a test to ensure built in artifacts have all necessary permissions declared either in required_permissions or implied_permissions

scudette and others added 6 commits May 20, 2025 17:21
8000
Client side artifacts run on the endpoint without ACL
enforcement. This is usually what we want but sometimes the extra
permissions can give the user extra permissions on the end point.

The artifact writer can specify required permissions which will be
enforced by the server prior to collecting the artifact.

However, sometimes the artifact can use plugins with high priviledge
safely - in that case we do not want to restrict the users that may
collect it.

For example say the artifact collects autoruns by shelling to the
autoruns.exe tool. Even though it is using the execve() plugin this
call is safe because the args are fixed and can not be influenced by
the user. However if the artifact passed user input into the execve()
plugin, the user requires the EXECVE permission.

Previously, the artifact could declare EXECVE as a required
permission, for artifact uses where user input was directly allowed in
execve() calls. This enforces additional checks on the launching user
to ensure they have the EXECVE permission.

This PR modifies the artifact verifier to track plugin permissions
used in the artifact. This allows us to see if the artifact
inadvertantly gives the user permissions they do not have.

This PR introduces another field to the artifact definition called
`implied_permissions` where the artifact writer can declare
permissions which the artifact will give but the user does not require
those. This helps the verifier identify additional permissions that
are accidentally given to users on the client.

This PR adds a test to ensure built in artifacts have all necessary
permissions declared either in `required_permissions` or
`implied_permissions`
Snyk has created this PR to upgrade webpack from 5.99.6 to 5.99.7.

See this package in npm:
webpack

See this project in Snyk:
https://app.snyk.io/org/scudette/project/76f4d127-566b-42ef-86f4-bdcbc92b90b4?utm_source=github&utm_medium=referral&page=upgrade-pr
Snyk has created this PR to upgrade axios from 1.8.4 to 1.9.0.

See this package in npm:
axios

See this project in Snyk:
https://app.snyk.io/org/scudette/project/76f4d127-566b-42ef-86f4-bdcbc92b90b4?utm_source=github&utm_medium=referral&page=upgrade-pr
Snyk has created this PR to upgrade ace-builds from 1.40.0 to 1.40.1.

See this package in npm:
ace-builds

See this project in Snyk:
https://app.snyk.io/org/scudette/project/76f4d127-566b-42ef-86f4-bdcbc92b90b4?utm_source=github&utm_medium=referral&page=upgrade-pr
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ scudette
❌ snyk-bot
You have signed the CLA already but the status is still pending? Let us recheck it.

@scudette scudette merged commit 9d5234e into master May 20, 2025
1 of 3 checks passed
@scudette scudette deleted the permissions branch May 20, 2025 11:42
scudette added a commit that referenced this pull request May 20, 2025
Client side artifacts run on the endpoint without ACL enforcement. This
is usually what we want but sometimes the extra permissions can give the
user extra permissions on the end point.

The artifact writer can specify required permissions which will be
enforced by the server prior to collecting the artifact.

However, sometimes the artifact can use plugins with high priviledge
safely - in that case we do not want to restrict the users that may
collect it.

For example say the artifact collects autoruns by shelling to the
autoruns.exe tool. Even though it is using the execve() plugin this call
is safe because the args are fixed and can not be influenced by the
user. However if the artifact passed user input into the execve()
plugin, the user requires the EXECVE permission.

Previously, the artifact could declare EXECVE as a required permission,
for artifact uses where user input was directly allowed in execve()
calls. This enforces additional checks on the launching user to ensure
they have the EXECVE permission.

This PR modifies the artifact verifier to track plugin permissions used
in the artifact. This allows us to see if the artifact inadvertantly
gives the user permissions they do not have.

This PR introduces another field to the artifact definition called
`implied_permissions` where the artifact writer can declare permissions
which the artifact will give but the user does not require those. This
helps the verifier identify additional permissions that are accidentally
given to users on the client.

This PR adds a test to ensure built in artifacts have all necessary
permissions declared either in `required_permissions` or
`implied_permissions`

---------

Co-authored-by: snyk-bot <snyk-bot@snyk.io>
scudette added a commit that referenced this pull request May 20, 2025
Client side artifacts run on the endpoint without ACL enforcement. This
is usually what we want but sometimes the extra permissions can give the
user extra permissions on the end point.

The artifact writer can specify required permissions which will be
enforced by the server prior to collecting the artifact.

However, sometimes the artifact can use plugins with high priviledge
safely - in that case we do not want to restrict the users that may
collect it.

For example say the artifact collects autoruns by shelling to the
autoruns.exe tool. Even though it is using the execve() plugin this call
is safe because the args are fixed and can not be influenced by the
user. However if the artifact passed user input into the execve()
plugin, the user requires the EXECVE permission.

Previously, the artifact could declare EXECVE as a required permission,
for artifact uses where user input was directly allowed in execve()
calls. This enforces additional checks on the launching user to ensure
they have the EXECVE permission.

This PR modifies the artifact verifier to track plugin permissions used
in the artifact. This allows us to see if the artifact inadvertantly
gives the user permissions they do not have.

This PR introduces another field to the artifact definition called
`implied_permissions` where the artifact writer can declare permissions
which the artifact will give but the user does not require those. This
helps the verifier identify additional permissions that are accidentally
given to users on the client.

This PR adds a test to ensure built in artifacts have all necessary
permissions declared either in `required_permissions` or
`implied_permissions`

---------

Co-authored-by: snyk-bot <snyk-bot@snyk.io>
scudette added a commit that referenced this pull request May 20, 2025
Client side artifacts run on the endpoint without ACL enforcement. This
is usually what we want but sometimes the extra permissions can give the
user extra permissions on the end point.

The artifact writer can specify required permissions which will be
enforced by the server prior to collecting the artifact.

However, sometimes the artifact can use plugins with high priviledge
safely - in that case we do not want to restrict the users that may
collect it.

For example say the artifact collects autoruns by shelling to the
autoruns.exe tool. Even though it is using the execve() plugin this call
is safe because the args are fixed and can not be influenced by the
user. However if the artifact passed user input into the execve()
plugin, the user requires the EXECVE permission.

Previously, the artifact could declare EXECVE as a required permission,
for artifact uses where user input was directly allowed in execve()
calls. This enforces additional checks on the launching user to ensure
they have the EXECVE permission.

This PR modifies the artifact verifier to track plugin permissions used
in the artifact. This allows us to see if the artifact inadvertantly
gives the user permissions they do not have.

This PR introduces another field to the artifact definition called
`implied_permissions` where the artifact writer can declare permissions
which the artifact will give but the user does not require those. This
helps the verifier identify additional permissions that are accidentally
given to users on the client.

This PR adds a test to ensure built in artifacts have all necessary
permissions declared either in `required_permissions` or
`implied_permissions`

---------

Co-authored-by: snyk-bot <snyk-bot@snyk.io>
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.

3 participants
0