BACnet icon indicating copy to clipboard operation
BACnet copied to clipboard

IndexOutOfRangeException when calling BacnetClient.ReadPropertyResponse

Open joproulx opened this issue 2 years ago • 0 comments

Issue

When calling ReadPropertyResponse, we get an IndexOutOfRangeException with the following stack trace:

System.IndexOutOfRangeException:
   at System.IO.BACnet.Serialize.EncodeBuffer.Add (BACnet, Version=2.0.4.0, Culture=neutral, PublicKeyToken=null)
   at System.IO.BACnet.Serialize.EncodeBuffer.Add (BACnet, Version=2.0.4.0, Culture=neutral, PublicKeyToken=null)
   at System.IO.BACnet.Serialize.ASN1.encode_application_object_id (BACnet, Version=2.0.4.0, Culture=neutral, PublicKeyToken=null)
   at System.IO.BACnet.Serialize.ASN1.bacapp_encode_application_data (BACnet, Version=2.0.4.0, Culture=neutral, PublicKeyToken=null)
   at System.IO.BACnet.Serialize.Services.EncodeReadPropertyAcknowledge (BACnet, Version=2.0.4.0, Culture=neutral, PublicKeyToken=null)
   at System.IO.BACnet.BacnetClient+<>c__DisplayClass347_0.<ReadPropertyResponse>b__1 (BACnet, Version=2.0.4.0, Culture=neutral, PublicKeyToken=null)
   at System.IO.BACnet.BacnetClient.SendComplexAck (BACnet, Version=2.0.4.0, Culture=neutral, PublicKeyToken=null)
   at System.IO.BACnet.BacnetClient+<>c__DisplayClass347_0.<ReadPropertyResponse>b__0 (BACnet, Version=2.0.4.0, Culture=neutral, PublicKeyToken=null)
   at System.IO.BACnet.BacnetClient.HandleSegmentationResponse (BACnet, Version=2.0.4.0, Culture=neutral, PublicKeyToken=null)
   at System.IO.BACnet.BacnetClient.ReadPropertyResponse (BACnet, Version=2.0.4.0, Culture=neutral, PublicKeyToken=null)

How to reproduce

  • ela-compil/BACnet v2.0.4
  • We use the library with the BacnetIpUdpProtocolTransport
  • When receiving a ReadPropertyRequest from a remote BACnet device, return a response bigger than 1476 bytes
  • The request must have been routed through a BACnet router

Investigation

From what we can see, because the request was coming from a BACnet router, the response need to add a bigger NPDU header and this would increase the max_offset of the buffer to a value higher than the actual size of the buffer as you can see here:

image

So when trying to add response payload (ADPU) bigger than 1476 bytes, the validation with the max_offset goes through even though the size of the buffer is not big enough and we get an IndexOutOfRangeException.

image

Potential fix

  • As a first workaround, we tried to increase the max payload in the transport (here). But this has the side effect of causing IP fragmentation because now the packets are too big.

  • We also tried another fix in BacnetClient.EncodeSegmentHeader. This seems to fix the issue from our initial tests: image

Couple of questions for you guys:

  • Does the fix makes sense?
  • Are there any tests we can run to validate we don't add regression to the library?

joproulx avatar Oct 19 '22 02:10 joproulx