Protocol Buffers always flags the same field number

[MaxwellBo]

Let's load a protoscope test file: https://github.com/protocolbuffers/protoscope/blob/main/testdata/message.pb

We detect field 114 here:

and here

and here

Let's try a file of my own - this time it thinks every field is 8.

[paxcut]

I can't imagine this pattern ever worked correctly. It uses global variables to display values that are calculated as the fie is read, but the values that are printed in the format functions will always be the last value the global takes. In the first example that value is 114 in your own example I don't really know by I venture that the last value that the global variable field_value takes is 8. An easy way to check this is to insert a print statement in the place where the global variable is being updated:

struct Key {
    std::print("{:#x}",$);
    type::uLEB128 keyDec;
    field_number = u32(keyDec) >> 3;
    std::print("{} {}",field_number, u32(keyDec) >> 3);
    wire_type = u32(keyDec) & 7;
}[[format("format_key")]];

where i also print the address of KeyDec and removed the [[sealed]] attribute. That prints the correct values one expects to see and if you scroll all the way to the bottom you will find the value it 'thinks' all the other ones are.
It should be easy to fix this but I know nothing of protobuffers so I don't know what else could be wrong.

[paxcut]

I think this is what the pattern should be doing. Can you test it to make sure the results are what you expect? If it does feel free to open a PR or I can do it if you prefer not to. Let me know one way or the other.

#pragma author WerWolv
#pragma description Google Protobuf wire encoding (.pb)

import std.core;
import std.io;
import std.mem;

import type.leb128;

struct ZigZag32 {
    u32 value;
} [[sealed, format("format_zigzag32")]];

fn format_zigzag32(ZigZag32 zigzag) {
    return s32((s32(zigzag.value) << 1) ^ (s32(zigzag.value) >> 31));
};

struct ZigZag64 {
    u64 value;
} [[sealed, format("format_zigzag64")]];

fn format_zigzag64(ZigZag64 zigzag) {
    return s64((s64(zigzag.value) << 1) ^ (s64(zigzag.value) >> 63));
};

enum WireType : u8 {
    Varint  		= 0,
    _64Bit  		= 1,
    LengthDelimited = 2,
    StartGroup  	= 3,
    EndGroup        = 4,
    _32Bit  		= 5
};


struct Key {
    type::uLEB128 keyDec;
    u32 field_number = u32(keyDec) >> 3;
    WireType wire_type = u32(keyDec) & 7;
}[[sealed, format("format_key")]];

fn format_key(Key keyObject) {
    return std::format("{} with field number {}", keyObject.wire_type, keyObject.field_number);
};

union _64Bit {
    u64 fixed64;
    ZigZag64 sfixed64;
    double dbl;
};

union _32Bit {
    u32 fixed32;
    ZigZag32 sfixed32;
    float flt;
};

struct LengthDelimited {
    type::uLEB128 length;
    char data[length];
};


struct Entry {
    Key key;

    if (key.wire_type == WireType::Varint)
        type::uLEB128 value;
    else if (key.wire_type == WireType::_64Bit)
        _64Bit value;
    else if (key.wire_type == WireType::LengthDelimited)
        LengthDelimited value;
    else if (key.wire_type == WireType::_32Bit)
        _32Bit value;
};

Entry entries[while(!std::mem::eof())] @ 0x00;

[MaxwellBo]

That is significantly better yes! All the field numberings are coming through properly. It's still not parsing submessages but that's something I will raise in a separate issue and isn't a regression from the status quo.

but the values that are printed in the format functions will always be the last value the global takes.

I can confirm that these field numbers were the last in each file.

It uses global variables to display values that are calculated as the fie is read, but the values that are printed in the format functions will always be the last value the global takes.

This was my first time reading the pattern files - that would have been this section here right?

ImHex-Patterns/patterns/protobuf.hexpat

Lines 35 to 37 in e779b88

	WireType wire_type;
	u32 tag;
	u32 field_number;

If it does feel free to open a PR or I can do it if you prefer not to. Let me know one way or the other.

Happy for you to raise - I don't think I will be able to adequately respond to any concerns that are raised in the PR review process. But I am encouraged to learn more about this pattern language for future issues!

Thank-you for your speedy reply and patch.

[paxcut]

That is significantly better yes! All the field numberings are coming through properly. It's still not parsing submessages but that's something I will raise in a separate issue and isn't a regression from the status quo.

Not only the field numberings but also the wire types were messed up.
If you know how to parse the sub-messages I can help writing the parts of the pattern that would do it. I have a good sense of the pattern language but not much on google's buffers. I would love to learn as they are powerful serializerers. So instead of raising an issue we could work on completing it ourselves.

I can confirm that these field numbers were the last in each file.

Makes sense and agrees with the observation that format functions are evaluated only at the end of the pattern file evaluation run.

This was my first time reading the pattern files - that would have been this section here right?
ImHex-Patterns/patterns/protobuf.hexpat
Lines 35 to 37 in e779b88
WireType wire_type;
u32 tag;
u32 field_number;

Yes, exactly those were the lines where the global variables were defined. I just made them local variables of the struct and accessed them from there using the . operator. If you know a little c/c++ you will find the pattern language easy to get started with. There are some fundamental differences between pattern programming and standard language programming but after a while you start to think about the representations in a different way.

Happy for you to raise - I don't think I will be able to adequately respond to any concerns that are raised in the PR review process. But I am encouraged to learn more about this pattern language for future issues!

Ok, I'm on it and please consider my offer on improving this one. Im available in the ImHex discord channel anytime.

Thank-you for your speedy reply and patch.

No problem and thank you for taking the time to help us make ImHex even better than it already is.

[applecuckoo]

thanks all, especially @paxcut!

Yeah, I was the one who implemented key decoding a while back, albeit without any first hand experience of protobuf... the main resource I relied on was this page though it's fairly obvious that I did make a few mistakes.

Having submessage support would be interesting - I did think about implementing this, but I came to the conclusion that there would be a chicken-and-egg problem between the Entry and LengthDelimited structs (i.e. callling Entry from within LengthDelimited). Trying that results in some eval_depth problems though... feedback is welcome!

[MaxwellBo]

@applecuckoo I think the somewhat annoying part about Protobuf is that, without schema support, the parse is ambiguous - a LEN tag precedes a field of some other message T, or string, or bytes, or a repeated ... [packed=true].

Is there anyway to "attempt" a parse of each these variants in the Pattern Language, and trying the next if the parse fails? This would equivalent of using the alt if you've ever used the Nom parser library.

@paxcut I'll take you up on that offer! I've joined the Discord - keen to find some time to figure this out.

@applecuckoo @paxcut I'll flag 2 things that may be of help:

1. Katai Struct's definition doesn't handle the recursion either
2. Protoscope heuristically tries to destructure the stream without schema support (like my suggestion above) but I have not investigated its implementation.

[WerWolv]

I have thought about this problem before too.
One way might be to just try to parse the content of these fields as a nested message and if the parse fails, simply interpret it as a byte array. This is possible currently using the following code:

try {
    Entry nestedEntry;
} catch {
    char data[length];
}

The thing with that is, the content of these data can be absolutely everything. So it might happen that the content just happens to be accidentally in the right format for it to parse as a message, it would just be complete bogus. Maybe having it always parse as a byte array but additionally also as a nested member (if possible) would be an option?

[paxcut]

Is there anyway to "attempt" a parse of each these variants in the Pattern Language, and trying the next if the parse fails? This would equivalent of using the alt if you've ever used the Nom parser library.

Pattern Language has a try {} catch{}that could deal with parsing failures.

@paxcut I'll take you up on that offer! I've joined the Discord - keen to find some time to figure this out.

Thats great!

@applecuckoo @paxcut I'll flag 2 things that may be of help:
Katai Struct's definition doesn't handle the recursion either
Protoscope heuristically tries to destructure the stream without schema support (like my suggestion above) but I have not investigated its implementation.

I didn't check Katai but I did check 010 and they dont handle recursion either.

[MaxwellBo]

@WerWolv

So it might happen that the content just happens to be accidentally in the right format for it to parse as a message, it would just be complete bogus.

Protobuf is vulnerable to this yes, and I have seen it happen on 2 seperate occasions. A few field numbers changed in a backwards incompatible manner and a message deserializer parsed gibberish into some of the fields.

If it's any consolation this is fortunately quite rare - it typically only happens for shorter messages where length delimiters improbably line up.