Initial linting of ddexec.sh

tinmarino commented 1 year ago

Hi @arget13,

It works super fine, nice script. I refactored a little, with the function declaration and docstring my style so that I can have a nice folding, I hope this is OK for you.

Otherwise, there are 2 minors improvement:

if [ -n "interp_off" ] (forgot the dollar)
bin variable is exported in my environment, so I get an "argument list too long" because this variable becomes very big and is passed (in my case as exported)

But most of all these are the changes:

[X] Quote expansions
[X] Use the default assignment idioms :=
[X] Nest all in function from top to bottom, create a main

Folding

The folding I get with docstrings inside functions, for reference

    1 #!/bin/sh
    2
    3 init_global(){                                                  : 'Init global variables                                                         > 64
   67 endian(){                                                       : 'Helper: Endian conversion'                                                     > 6
   73 sc_chunk(){                                                     : 'Exctract a chunk from the global SC_ARRAY'                                     > 6
   79 search_section(){                                               : 'Search for a section segment in file                                          > 87
  166 shellcode_loader(){                                             : '### TODO: SHF_COMPRESSED sections ###                                        > 158
  324 craft_stack(){                                                  : 'Craft initial stack                                                           > 75
  399 craft_shellcode(){                                              : 'Craft the shellcode to bootload user binary'                                  > 56
  455 ddexec(){                                                       : 'Main function'                                                               > 133
  588 ddexec "$@"

tinmarino commented 1 year ago

This would be all for this PR, just to note that I may create other one with these changes. What do you think of them?

[ ] Use array for command and not echo eval
[x] Comment
[ ] Assign a type to declaration, as there is much arithmetic
[x] Add automatic tests
[x] Reorganize and reorder functions: from top to bottom, hopefully a small main

arget13 commented 1 year ago

Hello! I'm really glad that someone actually took the time to read and understand such difficult to read code. Later I'll see in detail the PR. Regarding the next changes:

I would have used an array (it is the obvious answer!) but they aren't supported by ash (busybox).
About comments... I don't know, the script is pretty big already and this is supposed to be as small as possible in case you need to copy & paste through an unstable connection to a computer with restricted access to Internet.
Types? I don't think ash or any POSIX shell support them.
Automatic tests... yeah, I tried that but I'm really bad with github actions. You can see the configuration files for github actions in .github and you'll see that they don't work particularly well; if you want to fix that, that'd be great.
If you find a better way to organize the code, well, we can use it, of course.

arget13 commented 1 year ago

On the other hand please consider that I expect to change the technique to memdlopen. Whenever I have the time, hehe.

tinmarino commented 1 year ago

Hi @arget13,

Thank you for the fast response. I agree with your comments, and think the compatibility with all possible shell should not be broken (I guess it was hard to get, congratulation) let me show refs.

Array: no array allowed <= in effect, ash do not support them: Syntax error: "(" unexpected
Comment: no abusive comment, it is ok to add comment in some external .md files as you did (I love the README.md)
Types: not suported <= local: -i: bad variable name
Test: we all agree, I'll see that
Refactor: I do not think I'll improve: dividing in more function may create subshells or global variables.

So I'll just consider tests before you accept this small refactoring (all in functions) to avoid conflicts. Anyway it is the best next step and I'm educatively playing with GitHub Actions.

tinmarino commented 1 year ago

I'd like to collaborate more on this ddexec project and understand it is ok. Thank you for pointing me to memdlopen, I'll have a look at the paper (curiosity, i will not use it).

My interests

This leads me to introduce my interests. I am not in cybersecurity (any more), but rather in a user friendly TUI (git like) for remote execution on machines where I am invited. Usually for testing, with code in Bash on my machine piping to an ssh tunnel (this you are familiar lol).

Existing alternatives or friends

I created the lib_dispatch bash code to call any Bash function anywhere (with introspection) but it cannot call native binary encoded strings and calling some native code as mprocs would be a nice feature (my chiefs would appreciate :-)).

I do not like to touch filesystem either, not for furtivity as pentester but more because it may not exists, be in readonly, not mounted, slow, etc and this leads to add some magic (path) in code, may create some race conditions, etc => this is dirty even for legitimate code!

The only solution for in memory execution from shell I found before yours is using memfd_create syscall. See a recent response and also blog. This requires perl!

Brief

All that to say that the memory parsing you are doing in pure shell is really innovative, I was waiting for that! The features this unveil extend far beyond education and security. It empowers shell scripting and this is where my interest lie. I hope I can bring a little my added value (as shell expert).

Saludos desde Chile. See you at next (test) PR!

arget13 / DDexec