Rephrase custom PBA file descriptions in configuration #1027
Conversation
Codecov Report
@@ Coverage Diff @@
## release/1.10.0 #1027 +/- ##
==================================================
+ Coverage 27.63% 28.18% +0.54%
==================================================
Files 402 402
Lines 12838 12830 -8
==================================================
+ Hits 3548 3616 +68
+ Misses 9290 9214 -76
Continue to review full report at Codecov.
|
9399b86 to
4ea6ac1
Compare
DeepCode failed to analyze this pull requestSomething went wrong despite trying multiple times, sorry about that. |
| elif WormConfiguration.custom_PBA_windows_cmd: | ||
| self.command = WormConfiguration.custom_PBA_windows_cmd | ||
|
|
||
| def _execute_default(self): |
There was a problem hiding this comment.
Can you explain this change? It think it's not ideal to be downloading files from the island in the constructor if we can avoid it. Also, it makes sense to me that downloading the file is part of execution; I wouldn't expect the initialization of the PBA to fail if there were a network outage or other condition that prevented the download.
There was a problem hiding this comment.
Makes sense; fixed
There was a problem hiding this comment.
@shreyamalviya Can you rework and include the tests from #1020?
6e500f8 to
1020bd3
Compare
| @@ -0,0 +1,152 @@ | |||
| import pytest | |||
There was a problem hiding this comment.
Unit tests are nice, but not a silver bullet. There's an antipattern in TDD called the "mockery", when most of the functionality is mocked. I don't suggest refactoring the whole pba's to solve it, but I can see BB tests bringing more value here and still being trivial enough to implement (command execution at least). Other PBA's should be tested with BB's IMO.
| "description": "File to be uploaded after breaching. " | ||
| "If you want the file to be executed, " | ||
| "specify it in the 'Linux post breach command' field. " |
There was a problem hiding this comment.
| "description": "File to be uploaded after breaching. " | |
| "If you want the file to be executed, " | |
| "specify it in the 'Linux post breach command' field. " | |
| "description": "File will be uploaded after breaching. " | |
| "Use 'Linux post-breach command' field to " | |
| "change permissions, run or delete the file." |
There was a problem hiding this comment.
"File will be uploaded" doesn't seem right as a description. "File to be uploaded" is what I would expect.
| "description": "File to be uploaded after breaching. " | ||
| "If you want the file to be executed, " | ||
| "specify it in the 'Windows post breach command' field. " |
There was a problem hiding this comment.
| "description": "File to be uploaded after breaching. " | |
| "If you want the file to be executed, " | |
| "specify it in the 'Windows post breach command' field. " | |
| "description": "File will be uploaded after breaching. " | |
| "Use 'Windows post-breach command' field to " | |
| "run or delete the file." |
| @@ -35,9 +35,9 @@ | |||
| "title": "Windows post breach file", | |||
There was a problem hiding this comment.
| "title": "Windows post breach file", | |
| "title": "Windows post-breach file", |
| @@ -20,9 +20,9 @@ | |||
| "title": "Linux post breach file", | |||
There was a problem hiding this comment.
| "title": "Linux post breach file", | |
| "title": "Linux post-breach file", |
There was a problem hiding this comment.
We also need to change descriptions of PBA command fields. They should also contain examples. Linux description should be something like:
"Use this field to run custom commands or uploaded files on exploited machines. Example command chmod +x ./my_script.sh; ./my_script.sh ; rm ./my_script.sh"
1020bd3 to
bf4555e
Compare
bf4555e to
2b4fd9e
Compare
Related: #1020