undocumented/unintended ZYDIS_FORMATTER_FUNC_POST_OPERAND behavior

[440bx]

Hello,

Hooking the function for ZYDIS_FORMATTER_FUNC_POST_OPERAND does not seem to behave as documented. if the function returns ZYAN_STATUS_SUCCESS then the source operand is dropped from the instruction (not output/printed). if the function returns ZYAN_STATUS_FAILED then the source operand is printed.

The above behavior does not seem consistent with the behavior documented for ZydisFormatterFunc.

The above behavior can be obtained by making the following modifications to the "Formatter01" example.

(1.)

// add the following in the "data" array (as the first line)

0x48, 0x8D, 0x65, 0x00,   // lea rsp, [rbp]  // the ", [rbp]" will be dropped depending on return value

(2.)

// in DisassembleBuffer add the following statements

    default_post_op = (ZydisFormatterFunc) &ZydisFormatterPostOperand;
    ZydisFormatterSetHook(&formatter, ZYDIS_FORMATTER_FUNC_POST_OPERAND, (const void**) &default_post_op);

(3.) add the following hook function

static ZyanStatus ZydisFormatterPostOperand(const ZydisFormatter* formatter,
  ZydisFormatterBuffer* buffer,
  ZydisFormatterContext* context)
{   // breakpoint here - for tracing

    // there is no default function, therefore nothing to execute (nil pointer)

  // uncomment the statement below and the source operand will be printed 

  //return ZYAN_STATUS_FAILED;  // source operand is printed/output - the formatting process did
                              // not fail (contrary to what the documentation states)

  return ZYAN_STATUS_SUCCESS; // causes the source operand to be omitted
                              // this behavior is not documented (unintended ??)
}

(4.) declare the variable to pass to the set hook function

ZydisFormatterFunc default_post_op;

[flobernd]

Yes, we should clarify that in the documentation.

The proper way of omitting an operand here is to return ZYDIS_STATUS_SKIP_TOKEN.

A failure status code immediately stops the complete formatting and causes that code to be propagated to the exported ZydisFormatter* APIs.

[440bx]

What I find particularly disconcerting is that returning ZYAN_STATUS_SUCCESS causes the source operand to be dropped resulting in an invalid instruction being printed (missing the source operand.) When a function returns ZYAN_STATUS_SUCCESS, I expect a complete, valid, instruction not a mutilated one.

Basically, ZYAN_STATUS_SUCCESS is behaving sort of as ZYDIS_STATUS_SKIP_TOKEN. I say "sort of" because I didn't test returning ZYDIS_STATUS_SKIP_TOKEN and comparing the results but, after tracing the execution, I believe it might be the same.

[flobernd]

Right. There is a ! missing for the post-operand condition check.

[440bx]

The following question is strictly curiousity... I don't want it to be interpreted as being "unreasonable" or "pushy".

I see this bug as a fairly simple bug with a fairly simple fix, though I could be wrong about that but, presuming it is a simple fix, what's the reasonable time frame to expect a bug of this kind to be fixed ?

[flobernd]

If you create a PR, we can fix it as soon as you are ready 🙂

Should be fixed by adding the missing "not" in front of the ZYAN_SUCCESS() call after the post-operand callback in ZydisFormatterIntel.c and ZydisFormatterATT.c

Not sure if I have time to work on it myself over the weekend..

[440bx]

I'm not well versed in Zydis (but learning) nor github,

If I were, I would gladly make the necessary changes and test them but, at this time, I am too afraid I'll mess something up due to general "lack of knowledge" (read: ignorance.)