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

Add T1156 and T1504 attack techniques (shell startup file modifications) #687

Merged
merged 18 commits into from
Jun 24, 2020

Conversation

shreyamalviya
Copy link
Contributor

@shreyamalviya shreyamalviya commented Jun 13, 2020

Fixes #682 and fixes #686

T1156 and T1504 both aim at modifying shell startup files

TODO: Add technique for windows i.e. T1504 (modification of PowerShell profile)

@shreyamalviya shreyamalviya marked this pull request as ready for review June 15, 2020 13:06
Copy link
Contributor

@ShayNehmad ShayNehmad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a question in there. Also, what's the testing status? Have you tested this on Windows and Linux?

Comment on lines 1 to 13
BASH_STARTUP_FILES = ["~/.bashrc", "~/.profile", "~/.bash_profile"]


def get_linux_commands_to_modify_shell_startup_files():
return [
'echo \"# Succesfully modified {0}\"',
'3<{0} 3<&- |', # check for existence of file
'tee -a', # append to file
'{0}',
'&&',
'sed -i \'$d\' {0}', # remove last line of file
],\
BASH_STARTUP_FILES
Copy link
Contributor

@ShayNehmad ShayNehmad Jun 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will work in case the user is using bash, but what if the default shell is different (zsh etc.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The command will work but we'll have to add the startup file names for the other shells to BASH_STARTUP_FILES, then

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ShayNehmad Any reason to actually support all the shells?
sh, bash, dash. Those are standard?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@acepace If it's not difficult (adding paths of zsh, fish, dash, sh... ksh and tcsh maybe) then yeah, but if it requires a lot of research and testing (and not only adding paths to the STARTUP_FILES const) then it's not worth it.
@shreyamalviya your call :)

Copy link
Contributor Author

@shreyamalviya shreyamalviya Jun 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ShayNehmad @acepace it just requires adding the paths to the STARTUP_FILES constant but would it be a good idea to include other shells, given that the name of the technique is .bashrc and .bashprofile?

Maybe we could just exclude the info about the other shells from the ATT&CK report?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like your last sentence.

@shreyamalviya
Copy link
Contributor Author

Yep, tested on both!

Copy link
Contributor

@VakarisZ VakarisZ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job! But design can still be improved by splitting classes and methods into different level's of abstraction:
ModifyShellStartupFiles should get a list of ModifyShellStartupFile PBA objects and run it. No more.
ShellStartupPBAgenerator should create a list of ModifyShellStartupFile PBA objects. No more.
ModifyShellStartupFile PBA object should run a corresponding command, based on os. If windows command is empty and it's running on windows, it should do nothing.

SHELL_STARTUP_FILE = '$Profile'


def get_windows_commands_to_modify_shell_startup_files():
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we doing this through powershell commands rather than opening the file in python?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this was taken from atomic red team, where scripts are in powershell

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I change it to open it in python itself?

@acepace
Copy link
Contributor

acepace commented Jun 17, 2020

It's not clear to me where is the part where we're touching all users startup files rather than current user.
By doing this only for the current user, which may be root or a service user, we're unlikely to make a large impact.

@shreyamalviya shreyamalviya marked this pull request as draft June 20, 2020 19:22
@shreyamalviya shreyamalviya marked this pull request as ready for review June 22, 2020 16:08
@codecov
Copy link

codecov bot commented Jun 23, 2020

Codecov Report

Merging #687 into develop will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #687   +/-   ##
========================================
  Coverage    58.05%   58.06%           
========================================
  Files          138      139    +1     
  Lines         4482     4483    +1     
========================================
+ Hits          2602     2603    +1     
  Misses        1880     1880           
Impacted Files Coverage Δ
monkey/monkey_island/cc/services/config_schema.py 100.00% <ø> (ø)
monkey/__init__.py 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0ec5259...53e6f89. Read the comment docs.

Copy link
Contributor

@ShayNehmad ShayNehmad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's merge :) I'm OK with powershell commands instead of Python

(Accidentally force-pushed over the previous commit changing this)
@shreyamalviya shreyamalviya merged commit 0872730 into guardicore:develop Jun 24, 2020
@shreyamalviya shreyamalviya deleted the T1156 branch August 3, 2020 04:57
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.

Add "PowerShell Profile" attack technique (T1504) Add ".bash_profile and .bashrc" attack technique (T1156)
4 participants