Problem about limitation override in log_api function

[leeqwind]

Hi, jbremer.

I find a problem about log_api function in cuckoo monitor.

Perhaps the statement argnum--; in log_api function is faulty.

In g_explain_paramtypes array, i notice that there are some '!' characters in front of the real type character(such as 'b' or 'B') in some parameter type strings of some apis, I think it means that the parameter buffer should be recorded with no truncation.

But I find that there are some different places when processing with '!' character in log_explain and log_api functions.

log_explain function records every called api information(such as parameter names, parameter types, flag values and flag bitmasks, etc) before every time firstly recording the real information of the current call of the specific api.

In log_explain function, There is a for loop processing with the parameter type array and the parameter name array. I find at first it converts variable argnum (started from 2) to the char array argidx which will be used to mark the parameter index in bson, then it judges if the *fmt value is equal to '!', if it is true, the variable argnum will be decreased 1, then the current frame will be skipped and the for loop continues. Each time of the loop it makes variable fmt and argnum increase 1. It makes sure that the '!' character will not be recorded to the argument type bson array.

void log_explain(uint32_t index)
...
    for (uint32_t argnum = 2; *fmt != 0; argnum++, fmt++) {
        ultostr(argnum, argidx, 10);

        // Handle overrides.
        if(*fmt == '!') {
            argnum--;
            continue;
        }
...

In log_api function, I also find the similar processing. In the for loop processing with parameter list, at first argnum will be also converted to a char buffer idx, then argnum increases 1. Then, there is a judge of *fmt being equal to '!'. Equally, if it is true, variable override will be assigned to 1, and argnum decrease 1. Then fmt increase 1, and so on.

void log_api(uint32_t index, int is_success, uintptr_t return_value,
    uint64_t hash, last_error_t *lasterr, ...)
...
    int argnum = 2, override = 0;
    for (const char *fmt = sig_paramtypes(index); *fmt != 0; fmt++) {
        ultostr(argnum++, idx, 10);

        // Limitation override. Instead of displaying this right away in the
        // report we turn it into a buffer (much like the dropped files).
        if(*fmt == '!') {
            override = 1;
            argnum--;
            fmt++;
        }
...

Then there is the problem. In the case with '!' character in the parameter type string, in log_explain function, when *fmt == '!', argnum--, then it continues, and the for-loop will make both argnum and fmt increase 1. Then when fmt pointing the 'b' or 'B' character after '!', fmt has increased twice, and argnum has increased once; But in log_api function, when *fmt == '!', argnum--, fmt increase 1 instead of the "continue". It means that when fmt pointing the 'b' or 'B' character after '!', fmt has increased twice, and argnum has increased 0 time.

It causes a problem:

If there is an other parameter type character after the current 'b' or 'B' in the same parameter type string, the argnum of it will be duplicate with the argnum of 'b' or 'B'.

So is the statement argnum--; faulty?

I find that in most case of the type string with '!' character in g_explain_paramtypes array, "!b" or "!B" is at the end of the string, there is no character after "!b" or "!B" so the mistake in log_api function will not cause some practical problems.

But there are still some exceptions: the parameter type strings of CryptEncrypt, CryptProtectData, CryptProtectMemory, RtlCompressBuffer put the "!b" substring at the beginning of the parameter string, so that it is in front of the other common parameter type characters, then the problem happens.