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

modules: Check datastore ForceExploit before checking if session is root #17581

Merged
merged 3 commits into from Feb 2, 2023

Conversation

bcoles
Copy link
Contributor

@bcoles bcoles commented Feb 2, 2023

Currently, many local exploit modules check if the user has root permissions before proceeding, unless the operator has enabled ForceExploit.

if is_root? && !datastore['ForceExploit']
  fail_with(Failure::BadConfig, 'Session already has root privileges. Set ForceExploit to override.')
end

This is good; however, due to the order of conditions, when the operator has selected ForceExploit the module will still check first if the user has root permissions.

This is unnecessary. There is no reason to check for root permissions when ForceExploit is set. Additionally, as the is_root? check requires execution of multiple commands on the remote host, this check is significantly more expensive than a simple Boolean comparison with a local datastore option.

Worse, in instances where the module cannot determine whether the user has root permissions (ie, if id is not in PATH or returns unexpected output), the module will raise on Linux systems. An operator who wished to avoid this failure condition by forcing exploitation would not care if the root check failed - in fact, they may have enabled ForceExploit as a workaround specifically because the root check failed.

This PR modifies the conditions in 45 modules to check whether the operator has selected ForceExploit before checking permissions, which is slightly more efficient.

@bcoles bcoles added module code quality Improving code quality labels Feb 2, 2023
@adfoster-r7 adfoster-r7 added the rn-enhancement release notes enhancement label Feb 2, 2023
@adfoster-r7 adfoster-r7 merged commit 952a4fe into rapid7:master Feb 2, 2023
@adfoster-r7
Copy link
Contributor

adfoster-r7 commented Feb 2, 2023

Release Notes

This PR modifies the conditions in 45 local privilege escalation modules to check whether the operator set ForceExploit to true before checking the permissions required for exploitation on the remote target, which is more efficient and quieter over the network.

@bcoles bcoles deleted the modules-forceexploit branch February 2, 2023 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code quality Improving code quality module rn-enhancement release notes enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants