VARDESC: Premature invocation of _VARDESC.setType("lpvarValue")

[stmuecke]

Version of JNA: 5.15.0
Windows 10

While using TlbImp, I stumbled across a bug in VARDESC and _VARDESC. Constructors of both classes set the union's type to "lpvarValue", which can lead to an invalid memory access. This can be easily reproduced:

public class VarDescBug {

    public static void main(String[] args){
        TypeLibUtil libUtil = new TypeLibUtil("C:\\Windows\\System32\\stdole2.tlb");
        int count = libUtil.getTypeInfoCount();
        for (int i = 0; i < count; i++) {
            TypeInfoUtil infoUtil = new TypeInfoUtil(libUtil.getTypeInfo(i));
            TYPEATTR attr = infoUtil.getTypeAttr();
            for (int j = 0; j < attr.cVars.intValue(); j++) {
                try {
                    infoUtil.getVarDesc(j);
                } catch (Exception e) {
                    System.out.println("ERROR:" + e);
                }
            }
        }
    }

}

The problem can be fixed by changing all instances of setType("lpvarValue") to setType("oInst") and setting the union's type to "lpvarValue" only after checking VARDESC's varkind. For this we have to override the read() method in VARDESC:

@FieldOrder({"memid", "lpstrSchema", "_vardesc", "elemdescVar", "wVarFlags", "varkind"})
public class VARDESC extends Structure {

    [...]

    public VARDESC(Pointer pointer) {
        super(pointer);
        // REMOVE THIS LINE: this._vardesc.setType("lpvarValue");
        this.read();
    }
    
    @Override
    public void read() {
        super.read();
        if (varkind.value == VARKIND.VAR_CONST) {
            this._vardesc.setType("lpvarValue");
            readField("_vardesc");
        }
    }
    
}

[matthiasblaesing]

Agreed - this is a bug. I suggest that you create a PR for this and we get this fixed. Looking at the _VARDESC union, both the default constructor and the Pointer constructor make no sense. I would drop the setType and read calls there. As you correctly analysed read before the type is set from the outside makes no sense.

For the fix, yes overriding read in VARDESC looks like the right approach. Though I would first do an explicit read for varkind and then set the type for the union correctly and then issue the super.read() call.

[stmuecke]

Awesome reaction time. I didn't expect that. Thank you! PR created.