redballoonsecurity / ofrak

OFRAK: unpack, modify, and repack binaries.
https://ofrak.com
Other
1.87k stars 127 forks source link

Generate script in GUI based on user actions #265

Closed marczalik closed 1 year ago

marczalik commented 1 year ago

One sentence summary of this PR (This should go in the CHANGELOG!) While using the GUI, dynamically generate runnable OFRAK scripts based on corresponding user actions and allow user to view and download the current script.

Link to Related Issue(s) N/A

Please describe the changes in your request. Add ScriptBuilder class to handle generation of scripts. Add server endpoint for getting current script. Add ScriptView component to frontend to display the script.

Anyone you think should look at this, specifically? @whyitfor @Edward-Larson

Suggestions for Post-MVP Improvements

marczalik commented 1 year ago

@rbs-jacob @EdwardLarson one potential issue I'm seeing with the script is that, if the user does something that throws an error within OFRAK, that action is still recorded in the script. And because an error is returned, the script isn't updated automatically (the command will show up in the script when the next valid action runs). The easiest way to see this in action is to try and run StringFindReplaceModifier and replacing the first string with a longer string.

rbs-jacob commented 1 year ago

@rbs-jacob @EdwardLarson one potential issue I'm seeing with the script is that, if the user does something that throws an error within OFRAK, that action is still recorded in the script. And because an error is returned, the script isn't updated automatically (the command will show up in the script when the next valid action runs). The easiest way to see this in action is to try and run StringFindReplaceModifier and replacing the first string with a longer string.

Marc and I talked offline, and we agreed that the solution is to make sure the script builder is the absolute last thing that runs in each route before returning a response. That way, if there are any OFRAK actions that raise exceptions, the script builder will not add lines for them to the script.

For example, moving line 509 in the example below to after line 497 will fix the problem in that route.

https://github.com/redballoonsecurity/ofrak/blob/eeb6f9d9cb4ed178b0d30f16e81d43d63d93a2de/ofrak_core/ofrak/gui/server.py#L494-L510

One potential issue with this solution is that it may try to use selector based on features that are created after the action is run. For example, an analyzer that adds filename or virtual address attributes run before the script builder step may result in the script builder creating a selector that uses those attributes, even though the resource in the script won't have them until after the analyzer is run. It may be possible to address this by running the script builder first to queue changes, and only applying them after the action succeeds. It will require further discussion.

In the meantime, the change should be implemented for this PR, and we can fix outstanding bugs after.

EdwardLarson commented 1 year ago

@rbs-jacob Thanks for catching these issues. As a general solution to the problem of script generation errors blocking working GUI actions, we can refactor this code to only actually generate the script (including getting selectors etc.) when the user presses the "Generate(/View/Build) Script" button. I believe actions already are basically handled like this - it's add_variable that seems most error prone, and would need the most work to refactor.

marczalik commented 1 year ago

@dannyp303 @rbs-jacob @EdwardLarson I have removed the unused methods delete_action and get_all_of_type from ScriptBuilder since they are not being used in the MVP. If/when we need that functionality we can add it back in.