Possible bug in BacnetBase.cs?

[Lance Tollenaar]

Hi F. Chaxel.

I can across a funny scenario when talking to a BACnet device the other day. It seems that when some BACnet properties are provided by the controller, they can have a _NONE option. For example, decoding a BacnetPropertyIds.PROP_FAULT_PARAMETERS property, there is a BACnetFaultParameter struct with a type field, of datatype BACnetFaultType.

BACnetFaultType has a BACnetFaultType.NONE option. It looks like Yabe's default behaviour for handling this is:

public int ASN1decode(byte[] buffer, int offset, uint len_value)
{
...
        switch (type)
        {
                 case BACnetFaultType.FAULT_CHARACTERSTRING:
                 ...
                 break;
                 ...

                 default:
                        ** return len-1;**
        }
...
}

And in other cases it returns len - 1 as well.

However for the case where BACnetFaultType.NONE is returned by the controller, we return len - 1 which ends up being zero, but actually we should return len (which is 1 in this case) because we still need to parse the opening tag for the property using BACnetFaultType.

So the end result is an infinite loop further up the call stack, at:

while ((apdu_len - len) > 1)
{
...
}

And I get lots of weird errors when this happens, something like "There is an error in the XML document", and Yabe becomes unstable and sometimes crashes.

Does this sound right to you?

I think the fix for this in most cases is to return len instead of len - 1.

Cheers
Lance

[F. Chaxel]

Hi Lance,

I don't have any device to test it. So please check it and provide a solution if possible.

Maybe Christopher, who wrote this code, can do it too.

Bye.

[Lance Tollenaar]

Hi F. Chaxel,

I cannot possibly test all the different scenarios myself, but I could guess some of them and test others.

There are about 38 instances of return len - 1; in BacnetBase.cs. Here are the 3 scenarios of each:

1. The ASN1decode function has its own loop. The return len - 1; is correct as we detect an ending tag, return, then the outer function decodes the ending tag again.
2. There is a _NONE option in an enum somewhere which is actually still a valid BACnet item. So actually the tag is valid, but the code still returns len - 1, creating the infinite loop and crashing Yabe. This is the example I gave above, where the controller returns BACnetFaultType.NONE for an event enrolment object, because there is currently no Fault detected by the object.
3. The return len - 1; indicates a decode error, e.g. for an invalid tag number. In this case, I think the intended behaviour is that we should probably return -1; instead of return len - 1;, and this -1 will bubble up until we hit throw new Exception("Decode");, which I think is the correct behaviour. At the moment we are quietly returning 0 in this case and probably showing no error and just dropping the property from the PropertyGrid maybe?

Maybe I'll go through all 38 instances and deal with each one individually, soon.

Cheers

[Dennis V McEnaney]

I wonder if you could (easily) use the room-simulator (with changes) as a test-bed.

[Christopher Günther]

Problem with BACnetFaultParameter was I used BACnet Device Simulator from scada engine which hadn't implented alll scenarios yet; maybe now.

would adding fix it?

case BACnetFaultType.NONE:

                break;