Enhance the constraints for `argv` and `envp` #206

lebr0nli · 2023-07-08T08:59:43Z

Before this PR, one_gadget can't handle argv and envp well, especially when argv is on the stack.

For example, with libc 2.37 from http://archive.ubuntu.com/ubuntu/pool/main/g/glibc//libc6_2.37-0ubuntu2_amd64.deb, one_gadget might found the gadget like this:

But this is not true, the disassembly of this gadget is something like:

e2f95: 49 c7 47 10 00 00 00 mov QWORD PTR [r15+0x10],0x0
e2f9c: 00
e2f9d: 4c 89 ea mov rdx,r13
e2fa0: 4c 89 fe mov rsi,r15
e2fa3: 48 89 cf mov rdi,rcx
e2fa6: e8 75 f5 ff ff call e2520 <execve>
...
...
e3219: 48 8d 0d b2 1f 0d 00 lea rcx,[rip+0xd1fb2] # 1b51d2 <not_available+0x174>
e3220: 4c 89 65 b8 mov QWORD PTR [rbp-0x48],r12
e3224: 4c 8d 7d b0 lea r15,[rbp-0x50]
e3228: 48 89 4d b0 mov QWORD PTR [rbp-0x50],rcx
e322c: e9 64 fd ff ff jmp e2f95 <execvpe+0x275>

So there are several problems:

[rbp-0x50] doesn't need to be NULL, because it will eventually be set to rip+0xd1fb2(the address of "/bin/sh"). In addition, rbp-0x50 cannot be NULL because rbp-0x50 should be writable.
When execve is called, its argv will be [rbp-0x50]. [rbp-0x50] will be set to the address of "/bin/sh", [rbp-0x48] will be set to r12, and [rbp-0x40] will be set to NULL. So the correct constraint is that r12 need to be NULL or somehow {"/bin/sh", r12, NULL} is a valid argv (e.g. r12 points to address of "-").
When execve is called, r13 will become its envp. But [r13] or r13 doesn't always need to be NULL. Actually, the content of envp is not very important most of the time, if it's not NULL, we just need to make sure it's pointing to a readable address with some continuous readable addresses inside it that make r13 can become a valid envp.

After this PR, one_gadget will find the correct constraints for this gadget and show more useful information:

More before/after for the libc in http://archive.ubuntu.com/ubuntu/pool/main/g/glibc//libc6_2.37-0ubuntu2_amd64.deb:

Before:

After:

^ Similar to the first example, [rbp-0x50] is not control by user, the correct constraints are rax == NULL or {"/bin/sh", rax, NULL} is a valid argv.

Before:

After:

^ By showing more details, users can easier know that if they can control [rsp+0x70] to store a readable address(since sh won't bother by weird argv[0]) and luckily [rsp+0x78] is NULL, they can spawn the shell.

Not very sure if this PR fully fixes the problem that #120 mentioned, but hopefully it should at least fix the case when argv is on the stack for x86 libc.
Also, the enhancements and fixes introduced by this PR still have some TODOs (I mentioned them in the comment, e.g. the scoring parts).
Currently, I don't have a good idea for resolving them, let me know if you have any ideas that I can improve in this PR.
(Or maybe we can just leave them for now and fix them in another PR if they don't affect too much :p)

david942j · 2023-07-21T17:14:31Z

Thanks for this nice enhancement!

I haven't looked into the implementation yet, but you may want to resolve the tests failure first: https://github.com/david942j/one_gadget/actions/runs/5506576476/job/15242511295

Because your changes there seem to be more gadgets found.

lebr0nli · 2023-07-24T07:34:19Z

I haven't looked into the implementation yet, but you may want to resolve the tests failure first:

Ok, done.
I updated the tests and also fixed a bug I found while I examined some of the gadgets.
Please let me know if anything is missing!

lib/one_gadget/emulators/processor.rb

lib/one_gadget/emulators/aarch64.rb

lib/one_gadget/emulators/x86.rb

lib/one_gadget/fetchers/base.rb

david942j · 2023-07-28T13:20:13Z

lib/one_gadget/fetchers/base.rb

+
+      def check_nonstack_argv(arg, allow_null)
+        if allow_null
+          "[#{arg}] == NULL || #{arg} == NULL || #{arg} is a valid argv"


Can be simplified to "#{arg} == NULL || #{arg} is a valid argv" because [#{arg}] == NULL also means #{arg} is a valid argv

david942j · 2023-07-28T13:20:26Z

lib/one_gadget/fetchers/base.rb

+        if allow_null
+          "[#{arg}] == NULL || #{arg} == NULL || #{arg} is a valid argv"
+        else
+          "[#{arg}] == NULL || #{arg} is a valid argv"


can simply be #{arg} is a valid argv

david942j · 2023-07-28T13:22:11Z

lib/one_gadget/fetchers/base.rb

+          # TODO: Handle the case when libc will write something into envp
+          cons = global_var?(envp[0]) ? nil : "#{arg} == NULL || {#{envp.join(', ')}, ...} is a valid envp"
+        else
+          cons = "[#{arg}] == NULL || #{arg} == NULL || #{arg} is a valid envp"


"#{arg} == NULL || #{arg} is a valid envp"

david942j · 2023-07-28T13:25:08Z

lib/one_gadget/gadget.rb

@@ -81,6 +83,7 @@ def calculate_score(expr)
        when /^writable/ then calculate_writable_score(expr.sub('writable: ', ''))
        when / == NULL$/ then calculate_null_score(expr.sub(' == NULL', ''))
        when / <= 0$/ then calculate_null_score(expr.sub(' <= 0', ''))
+        when / is a valid (argv|envp)$/ then 0.2 # TODO: Calculate the score more precisely


I feel we can give it score 1, since it basically means something on stack is NULL (same as the case "[sp+xx] == NULL is easy" in calculate_null_score)

I have some reservations about assigning a score of 1.
Is it truly correct to attribute such a high score to it? This is particularly relevant when we examine cases like {"sh", "-c", x, NULL}.
In such instances, the value of x must hold a readable and contextually meaningful address.
However, manipulating x to an address that meets these criteria might not easy to be feasible most of the time (?).
Furthermore, when addressing cases like {"/bin/sh", x, NULL}, the score attributed to this constraint should probably equal to or less than the score derived from calculate_null_score(x), instead of always 1.

To address the scenario I mentioned earlier, my initial approach involved creating a set of comprehensive conditions, such as "[#{arg}] == NULL || ... is a valid argv".
By assigning a relatively low score of 0.2 to the constraint for is valid argv, the dominant influence within the entire || constraint becomes [#{arg}] == NULL for the majority of cases.
The is valid argv constraint essentially serves as supplementary information and acts as a fallback solution. Its significance truly emerges when our attempts to satisfy all other constraints have proven unsuccessful.

Nevertheless, my approach has given rise to an additional predicament, as can be gleaned from this instance: #206 (comment).

So, I think we have the following options:

Simplify the constraints when we generated it, i.e. directly fixing somewhere like this: Enhance the constraints for argv and envp #206 (comment), and re-parsing the ... in ... is a valid argv/envp for calculating the more accurate score here.

Keep this for now, until we want more accurate score :). (I personally prefer this option haha. Even if we make the score more accurate, it probably doesn't have a difference in the same output level. In addition, I found the verbose constraints like [r12] == NULL || r12 is a valid envp to be more intuitive than a single r12 is a valid envp, although [r12] == NULL is redundancy.)

What do you think?

Thanks for the explanation and I am convinced it's different from the NULL cases indeed. Let's go with current implementation and also remove TODO: Calculate the score more precisely, as you said I don't think it's worthy to to enhance it. Instead, add a comment that "/ is a valid (argv|envp)$/" usually means the register has to be a readable and pointer.

Ok, no problem!

Before this commit, one_gadget can't handle `argv` and `envp` well, especially when `argv` or `envp` is on the stack. For example, with libc 2.37 from http://archive.ubuntu.com/ubuntu/pool/main/g/glibc//libc6_2.37-0ubuntu2_amd64.deb , one_gadget might found gadget like this: ``` 0xe3219 execve("/bin/sh", rbp-0x50, r13) constraints: address rbp-0x48 is writable [rbp-0x50] == NULL || rbp-0x50 == NULL [r13] == NULL || r13 == NULL ``` But this is not true, the disassembly of this gadget is something like: ``` e2f95: 49 c7 47 10 00 00 00 mov QWORD PTR [r15+0x10],0x0 e2f9c: 00 e2f9d: 4c 89 ea mov rdx,r13 e2fa0: 4c 89 fe mov rsi,r15 e2fa3: 48 89 cf mov rdi,rcx e2fa6: e8 75 f5 ff ff call e2520 <execve> ... ... e3219: 48 8d 0d b2 1f 0d 00 lea rcx,[rip+0xd1fb2] # 1b51d2 <not_available+0x174> e3220: 4c 89 65 b8 mov QWORD PTR [rbp-0x48],r12 e3224: 4c 8d 7d b0 lea r15,[rbp-0x50] e3228: 48 89 4d b0 mov QWORD PTR [rbp-0x50],rcx e322c: e9 64 fd ff ff jmp e2f95 <execvpe+0x275> ``` So there are several problems: 1. `[rbp-0x50]` doesn't need to be `NULL`, because it will eventually be set to `rip+0xd1fb2`(the address of "/bin/sh"). In addition, `rbp-0x50` cannot be `NULL` because `rbp-0x50` should be writable. 2. When `execve` is called, its argv will be `[rbp-0x50]`. `[rbp-0x50]` will be set to the address of "/bin/sh", `[rbp-0x48]` will be set to `r12`, and `[rbp-0x40]` will be set to `NULL`. So the correct constraint is that `r12` need to be NULL or somehow `{"/bin/sh", r12, NULL}` is a valid argv (e.g. `r12` points to address of "-"). 3. When `execve` is called, `r13` will become its envp. But `[r13]` or `r13` doesn't always need to be `NULL`. Actually, the content of `envp` is not very important most of the time, if it's not `NULL`, we just need to make sure it's pointing to a readable address with some continuous readable addresses inside it that make r13 can become a valid envp. After this commit, one_gadget will find the correct constraints for this gadget and show more useful information: ``` 0xe3219 execve("/bin/sh", rbp-0x50, r13) constraints: address rbp-0x48 is writable r12 == NULL || {"/bin/sh", r12, NULL} is a valid argv [r13] == NULL || r13 == NULL || r13 is a valid envp ```

See david942j#206 (comment) for some discussions/explanations about this.

david942j · 2023-08-16T15:02:38Z

Merged, thanks a lot for the contribution!

lebr0nli · 2023-11-28T03:32:17Z

This patch seemed to work well during the recent CTF challenge I played, and I have actually seen someone fail to solve the challenge because they get misled by the old version one_gadget lol.
Should we bump the version or maybe make a beta release?

Also seems we need to update the gadgets/constraints saved on the remote(?), seems like I still need to add a -f flag to avoid one_gadget fetching the wrong constraints from the remote sometimes.

I can probably help if I have some time(if needed :p), could you give me some guides to helping maintain these?
Thanks!

david942j · 2023-11-29T15:36:26Z

remote actually means on github, let me do the update and release a new version

david942j · 2023-11-29T16:15:55Z

v1.9.0 released

lebr0nli · 2023-11-29T23:48:31Z

Thanks!

lebr0nli force-pushed the enhance-argv-envp-constraints branch from 580a65d to 47b6911 Compare July 9, 2023 05:25

lebr0nli force-pushed the enhance-argv-envp-constraints branch from eaf91d0 to 654bba2 Compare July 25, 2023 00:56

david942j requested changes Jul 28, 2023

View reviewed changes

lebr0nli added 7 commits August 13, 2023 16:47

Refactoring to make the code cleaner

c5e5d00

Fix some offenses

7b230eb

Fix potential bugs for envp on the stack

b44628b

Refined gadget scoring for argv and envp

a8c50ba

Fix the case that argv[0] is not sh but argv[1] is -c

5e97dfc

Update the tests

dac0f72

lebr0nli force-pushed the enhance-argv-envp-constraints branch from 654bba2 to dac0f72 Compare August 13, 2023 08:49

lebr0nli added 4 commits August 13, 2023 16:52

Update the comments

5d723c6

Add documents for get_corresponding_stack(obj)

542324d

Fix typo

927df0b

Move some comments into generate_argv_with_sh

683f20e

lebr0nli force-pushed the enhance-argv-envp-constraints branch from 6c229b2 to 683f20e Compare August 13, 2023 10:33

lebr0nli added 2 commits August 13, 2023 18:45

Add a TODO for getting the true content of argv[1]

e445c74

Update the comment of gadget scoring for argv and envp

fb886b7

See david942j#206 (comment) for some discussions/explanations about this.

david942j merged commit e2419f7 into david942j:master Aug 16, 2023

This was referenced Aug 5, 2024

Consider the content of argv array as constraints #120

Closed

Feature: consider register with controllable content of pointer as gadgets' constraint #8

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance the constraints for `argv` and `envp` #206

Enhance the constraints for `argv` and `envp` #206

lebr0nli commented Jul 8, 2023 •

edited

Loading

david942j commented Jul 21, 2023

lebr0nli commented Jul 24, 2023

david942j Jul 28, 2023

david942j Jul 28, 2023

david942j Jul 28, 2023

david942j Jul 28, 2023

lebr0nli Aug 13, 2023

david942j Aug 14, 2023

lebr0nli Aug 15, 2023

david942j commented Aug 16, 2023

lebr0nli commented Nov 28, 2023 •

edited

Loading

david942j commented Nov 29, 2023

david942j commented Nov 29, 2023

lebr0nli commented Nov 29, 2023

Enhance the constraints for argv and envp #206

Enhance the constraints for argv and envp #206

Conversation

lebr0nli commented Jul 8, 2023 • edited Loading

david942j commented Jul 21, 2023

lebr0nli commented Jul 24, 2023

david942j Jul 28, 2023

Choose a reason for hiding this comment

david942j Jul 28, 2023

Choose a reason for hiding this comment

david942j Jul 28, 2023

Choose a reason for hiding this comment

david942j Jul 28, 2023

Choose a reason for hiding this comment

lebr0nli Aug 13, 2023

Choose a reason for hiding this comment

david942j Aug 14, 2023

Choose a reason for hiding this comment

lebr0nli Aug 15, 2023

Choose a reason for hiding this comment

david942j commented Aug 16, 2023

lebr0nli commented Nov 28, 2023 • edited Loading

david942j commented Nov 29, 2023

david942j commented Nov 29, 2023

lebr0nli commented Nov 29, 2023

Enhance the constraints for `argv` and `envp` #206

Enhance the constraints for `argv` and `envp` #206

lebr0nli commented Jul 8, 2023 •

edited

Loading

lebr0nli commented Nov 28, 2023 •

edited

Loading