5
\$\begingroup\$

I've written the following code in assembly, which I then convert to machine opcodes and inject into a process with VirtualAlloc at runtime. This allows me to circumvent a restriction of vba:

mov rax, QWORD PTR [rcx+0x8]              # gets the refcount
dec rax                                   # decrement refCount
mov QWORD PTR [rcx+0x8], rax              # update refCount in obj
cmp rax, 0x0                              # compare refCount with 0
je +0x1                                   # jump if refCount is zero to release section
ret                                       # early return from function
# jumps to here - IUnknown::Release of child object if present
mov QWORD PTR [rsp+0x08], rcx             # cache RCX
mov rcx, QWORD PTR [rcx+0x10]             # read the child object into RCX
cmp rcx, 0x0                              # ensure the child object hasn't been set to Nothing
je +0xD                                   # jump if child object is Nothing
mov rax, QWORD PTR [rcx]                  # get vtable pointer from [RCX]
mov rax, QWORD PTR [rax+0x10]             # get IUnknown::Release function pointer (3rd vtable entry = (3-1) * 0x08 = 0x10)
sub rsp, 0x28                             # allocate shadow space; 0x20 + 0x08 pad to keep 16 bit aligned when + 0x08 call return address
call rax                                  # call the function pointed to by RAX
# jumps to here - CoTaskMemFree section
mov rcx, QWORD PTR [rsp+0x30]             # restore RCX to the parent object (remembering to jump over the shadow space)
movabs rax, 0x8877665544332211            # move addrCoTaskMemFree into RAX
call rax                                  # call the function pointed to by RAX
add rsp, 0x28                             # deallocate shadow space
xor rax, rax                              # set RAX to 0
ret                                       # return from function

The purpose of this code is a 64 bit Windows Intel implementation of an object's IUnknown::Release method (docs). Here I have a parent object and a child object, and this is the parent's release method.

IUnknown::Release should:

  • decrement a refcount
  • if the refcount is 0 call IUnknown::Release on any member objects
  • call CoTaskMemFree on the parent object instance
  • return the refcount after release

For reference the memory layout of the parent object is this:

Offset Value size
0x00 vtablePointer 0x08 - 64 bits
0x08 refcount 0x08 - 64 bits
0x10 **childVTable 0x08 - 64 bits

In my programme the assembly code is stored as hexadecimal machine code with a placeholder for the CoTaskMemFree function address that is inserted at runtime.

Debugging this is very tedious, the assembly code is a thunk in VBA injected into memory at runtime, and so I have to attach to Excel.exe with x64dbg, create a breakpoint at the memory allocated for the function code, inject it and wait for the breakpoint to be hit. VBA is interpreted so the callstack is very confusing. As this code is injected I also don't think I can use jump labels in assembly and need to hardcode jump relative offsets which is error prone. Unless there is an assembler or linker flag that does this for me and enforces relative jumps. JIT assembled code surely works this way?

The code works - but I have had to teach myself assembly programming from scratch for this project so I am not sure if it is well written or follows good conventions. I would like any feedback at all.

\$\endgroup\$

2 Answers 2

Reset to default
4
\$\begingroup\$

The code works ?

Except if the child object happens to be Nothing, because then your code will not have allocated the shadow space and thus the instructions at the CoTaskMemFree section will treat the stack memory wrongly!
The solution is to bring the sub rsp, 0x28 instruction to just before the test for Nothing. Make sure you don't forget to adjust the hardcoded jump offset!

Assembly style

You are mentioning QWORD PTR in cases where the assembler already knows the size from the register involved. eg. Better change mov rax, QWORD PTR [rcx+0x8] into mov rax, [rcx + 8].

You have written comments about things that are obvious:

mov rax, QWORD PTR [rcx+0x8]    # gets the refcount
dec rax                         # decrement refCount
mov QWORD PTR [rcx+0x8], rax    # update refCount in obj
cmp rax, 0x0                    # compare refCount with 0

It does not add anything useful to comment about decrementing, updating, and comparing the refcount as that is already obvious from looking at these dec, mov, and cmp instructions.

In all, the program text 'looks' heavy! You can make it a lot easier for the reviewer to read if you would:

  • drop some redundant comments
  • insert some more blank lines
  • use a tabular format where the instruction's operands get aligned to their own column
  • not mention QWORD PTR where it is not strictly needed
  • have whitespace around the '+' operator
mov    rax, [rcx + 0x8]        # gets the refcount
dec    rax
mov    [rcx + 0x8], rax
cmp    rax, 0x0
je     +0x1                    # jump if refCount is zero to release section
ret                            # early return from function

# jumps to here - IUnknown::Release of child object if present
mov    [rsp + 0x08], rcx       # cache RCX
mov    rcx, [rcx + 0x10]       # read the child object into RCX
cmp    rcx, 0x0                # ensure the child object hasn't been set to Nothing
je     +0xD                    # jump if child object is Nothing
mov    rax, [rcx]              # get vtable pointer from [RCX]
mov    rax, [rax + 0x10]       # get IUnknown::Release function pointer (3rd vtable entry = (3-1) * 0x08 = 0x10) (a)
sub    rsp, 0x28               # allocate shadow space
call   rax

# jumps to here - CoTaskMemFree section
mov    rcx, [rsp + 0x30]       # restore RCX to the parent object
movabs rax, 0x8877665544332211 # move addrCoTaskMemFree into RAX (b)
call   rax
add    rsp, 0x28               # deallocate shadow space
xor    rax, rax
ret                            # return from function

(a) Have mercy on the reader that does not have a super wide screen. Split extra long comments so they span more than one line, without disrupting the nice tabular format.
(b) I would improve this comment further emphasizing 0x8877665544332211 is indeed a placeholder.

Assembly do's

The xor rax, rax can be replaced by the 1 byte shorter xor eax, eax that also will empty the full 64 bits of RAX.

The cmp rcx, 0x0 (that ensures the child object hasn't been set to Nothing) is better written as test rcx, rcx (shorter code) and then followed by jz +0xD instead of je +0xD since using jz is more idiomatic following a test instruction.

The cmp rax, 0x0 on the 4th line can be omitted as the dec rax on the 2nd line already defines the necessary zero flag for your je +0x1. Again, more idiomatic to use jz +0x1 following that dec instruction.

mov    rax, [rcx + 0x8]        # gets the refcount
dec    rax
mov    [rcx + 0x8], rax
jz     +0x1                    # jump if refCount is zero to release section
ret                            # early return from function

# jumps to here - IUnknown::Release of child object if present
mov    [rsp + 0x08], rcx       # cache RCX
mov    rcx, [rcx + 0x10]       # read the child object into RCX
test   rcx, rcx                # ensure the child object hasn't been set to Nothing
jz     +0xD                    # jump if child object is Nothing
mov    rax, [rcx]              # get vtable pointer from [RCX]
mov    rax, [rax + 0x10]       # get IUnknown::Release function pointer
                               # (3rd vtable entry = (3-1) * 0x08 = 0x10)
sub    rsp, 0x28               # allocate shadow space
call   rax

# jumps to here - CoTaskMemFree section
mov    rcx, [rsp + 0x30]       # restore RCX to the parent object
movabs rax, 0x8877665544332211 # move placeholder for addrCoTaskMemFree into RAX
call   rax
add    rsp, 0x28               # deallocate shadow space
xor    eax, eax
ret                            # return from function

since real assemblers don't preserve my relative jumps, they assume a linker step coming up...

FASM is the ideal tool for what you are coding

It does not require the use of a separate linker, and by default, when there is no format directive in the source file, the flat assembler simply puts generated instruction codes into the output, creating this way a binary file. By default FASM generates 16-bit code, but you can turn it into 64-bit mode by using the use64 directive. All output code is always in the order in which it was entered into the source file.

You no longer need to hardcode the jump offsets. Just use regular labels. The only other change is replacing # by ; for the code comments.

Compiling from within the FASM IDE produces a binary file of 65 bytes.

SOURCE

use64

mov    rax, [rcx + 0x8]        ; gets the refcount
dec    rax
mov    [rcx + 0x8], rax
jz     Release                 ; jump if refCount is zero to release section
ret                            ; early return from function

Release:                       ; IUnknown::Release of child object if present
mov    [rsp + 0x08], rcx       ; cache RCX
mov    rcx, [rcx + 0x10]       ; read the child object into RCX
sub    rsp, 0x28               ; allocate shadow space
test   rcx, rcx                ; ensure the child object hasn't been set to Nothing
jz     Nothing                 ; jump if child object is Nothing
mov    rax, [rcx]              ; get vtable pointer from [RCX]
mov    rax, [rax + 0x10]       ; get IUnknown::Release function pointer
call   rax                     ; (3rd vtable entry = (3-1) * 0x08 = 0x10)

Nothing:                       ; CoTaskMemFree section
mov    rcx, [rsp + 0x30]       ; restore RCX to the parent object
mov    rax, 0x8877665544332211 ; move placeholder for addrCoTaskMemFree into RAX
call   rax
add    rsp, 0x28               ; deallocate shadow space
xor    eax, eax
ret                            ; return from function

LISTING

But FASM also comes with a listing tool that you run from a CMD prompt:

C:\FASMW>fasm inject.asm inject.bin -s inject.fas
C:\FASMW>listing inject.fas inject.lst

The offset addresses make it very easy to locate the placeholder for addrCoTaskMemFree.

                                                                use64
                                                                
00000000: 48 8B 41 08                                           mov    rax, [rcx + 0x8]        ; gets the refcount
00000004: 48 FF C8                                              dec    rax
00000007: 48 89 41 08                                           mov    [rcx + 0x8], rax
0000000B: 74 01                                                 jz     Release                 ; jump if refCount is zero to release section
0000000D: C3                                                    ret                            ; early return from function
                                                                
                                                                Release:                       ; IUnknown::Release of child object if present
0000000E: 48 89 4C 24 08                                        mov    [rsp + 0x08], rcx       ; cache RCX
00000013: 48 8B 49 10                                           mov    rcx, [rcx + 0x10]       ; read the child object into RCX
00000017: 48 83 EC 28                                           sub    rsp, 0x28               ; allocate shadow space
0000001B: 48 85 C9                                              test   rcx, rcx                ; ensure the child object hasn't been set to Nothing
0000001E: 74 09                                                 jz     Nothing                 ; jump if child object is Nothing
00000020: 48 8B 01                                              mov    rax, [rcx]              ; get vtable pointer from [RCX]
00000023: 48 8B 40 10                                           mov    rax, [rax + 0x10]       ; get IUnknown::Release function pointer
00000027: FF D0                                                 call   rax                     ; (3rd vtable entry = (3-1) * 0x08 = 0x10)
                                                                
                                                                Nothing:                       ; CoTaskMemFree section
00000029: 48 8B 4C 24 30                                        mov    rcx, [rsp + 0x30]       ; restore RCX to the parent object
0000002E: 48 B8 11 22 33 44 55 66 77 88                         mov    rax, 0x8877665544332211 ; move placeholder for addrCoTaskMemFree into RAX
00000038: FF D0                                                 call   rax
0000003A: 48 83 C4 28                                           add    rsp, 0x28               ; deallocate shadow space
0000003E: 31 C0                                                 xor    eax, eax
00000040: C3                                                    ret                            ; return from function
\$\endgroup\$
5
  • 2
    \$\begingroup\$ Thanks for great feedback, the formatting and assembly do's were exactly what I was after and the bug catch is a good spot! A few questions if that's okay 1) what formatter did you use to get that indentation - I have been doing it by hand and it's a faff. I'm embarrassed to say I've been using defuse.ca/online-x86-assembler.htm#disassembly and chatgpt to convert to opcodes since real assemblers don't preserve my relative jumps, they assume a linker step coming up... \$\endgroup\$ Commented Jul 22, 2024 at 8:41
  • \$\begingroup\$ 2) I've noticed a lot of functions using push RBP; mov RBP, rsp; [...] pop rbp or something to set up a stack frame I think it's called, I haven't done that but maybe I should? I have also read you are supposed to use "unwind codes" in case of exceptions (which release should not ever raise so maybe this is a non issue) and read something about a "red zone" but I'm not sure if this is necessary or if it is then what advice you would give for the context of runtime injected hand-written assembly as it seems very fiddly to get correct by hand? \$\endgroup\$ Commented Jul 22, 2024 at 8:47
  • \$\begingroup\$ @Greedo Recommend using jump labels rather than hand-calculated offsets if at all possible. \$\endgroup\$ Commented Jul 23, 2024 at 2:45
  • \$\begingroup\$ @Greedo The way you allocate what you call “shadow space” is probably more efficient than push/pop, unless all you need to do is save and restore a few registers. You aren’t expected to preserve rcx, under the x64 calling convention. \$\endgroup\$ Commented Jul 23, 2024 at 2:49
  • \$\begingroup\$ @Greedo FASM (available at flatassembler.net) might be the right tool for your project. \$\endgroup\$ Commented Jul 28, 2024 at 15:21
0
\$\begingroup\$

If Possible, Use Inline Assembly or MASM

You should be able to call a function written in either inline assembly or 64-bit MASM from an IUnknown::Release stub written in a higher-level language.

Stack Alignment

This is not a leaf function, since you make function calls from it. Under the x64 calling convention, the stack must remain aligned on a 16-byte boundary. However, you subtract 28 bytes. This must be increased to 32 or decreased to 16.

You save and restore rcx on the stack, but you are not expected to preserve this register. To preserve it across a function call, you could save it to a register that the called function preserves. If you only need a local copy, you could use another local register you don’t need to preserve, such as r9.

Labels

I highly recommend replacing the hand-calculated jump offsets with labels if at all possible.

You should also use the label CoTaskMemFree if available. (It should be in OLE32.lib, or, if loaded from a DLL, you can save the function pointer in a global variable with GetProcAddress).

\$\endgroup\$
6
  • \$\begingroup\$ Thanks for the review. I think the point on MASM yes I agree in principle but I'm not sure if you've accounted for the context in the question/ would love some clarification on how I would integrate better tooling? Specifically the "higher order language" I'm calling the assembly from is VBA which is not compiled and certainly has no inline asm which is why I'm injecting some. You should see the whole framework I've built just to get to the point of executing asm from VBA (yuk). But maybe I could use masm -if it had an output format that enforced relative jumps... \$\endgroup\$ Commented Jul 23, 2024 at 9:10
  • \$\begingroup\$ ... Since the assembly binary blob needs to be injected at runtime (as VBA is interpreted language. \$\endgroup\$ Commented Jul 23, 2024 at 9:10
  • \$\begingroup\$ 2) on 16 byte alignment, I understood the return address pushed to the stack by call is included in the calculation - so 0x28 bytes reserved + 0x8 byte return address = 0x30 which is aligned. 0x28 is used instead of 0x8 or 0x18 since the windows abi requires 0x20 reserved at least for the 4 argument parameters by convention. Is my understanding correct? Or do you ignore the call return address in the stack alignment 3) r9 I already made this mistake, since this is not a leaf function, the child function call can clobber those registers by using them itself IIUC \$\endgroup\$ Commented Jul 23, 2024 at 9:16
  • \$\begingroup\$ ... And any registers preserved by the called function, I would also have to preserve and wind up using the stack in the exact same way. Unless I'm missing something you mean (which is more than likely -in which case could you possibly expand the point with a code escape, if you were willing?) \$\endgroup\$ Commented Jul 23, 2024 at 9:19
  • \$\begingroup\$ @Greedo Good point about the return address being pushed to the stack. I goofed. I believe VBA can access objects implemented in other languages, through CreateObject(). I don’t know the requirements of your framework, but you appear to be using an ActiveX interface. \$\endgroup\$ Commented Jul 23, 2024 at 17:51

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.