After double checking this one, it looks like I mistakenly thought decode_unsigned16 returned the decoded value, but it actually returns the number of bytes consumed. I'm going to leave this one open since I am worried about there being some hidden arithmetic overflows, but wanted to provide some updated thoughts. I'll do another pass and update the ticket this week.
After double checking this one, it looks like I mistakenly thought decode_unsigned16 returned the decoded value, but it actually returns the number of bytes consumed. I'm going to leave this one open since I am worried about there being some hidden arithmetic overflows, but wanted to provide some updated thoughts.
Multiple out-of-bounds accesses in bacerror code paths
Another caveat of this function that I'm sure you're aware of: the length value is MAX_NPDU when called through npdu_decode (such as in router-mstp). int npdu_decode(uint8_t *npdu, BACNET_ADDRESS *dest, BACNET_ADDRESS *src, BACNET_NPDU_DATA *npdu_data) { return bacnet_npdu_decode(npdu, MAX_NPDU, dest, src, npdu_data); } I'm assuming this is for compatibility, but wanted to point out the unsafe code path.
Another caveat of this function that I'm sure you're aware of: the length value is MAX_NPDU when called through npdu_decode (such as in router-mstp). int npdu_decode(uint8_t *npdu, BACNET_ADDRESS *dest, BACNET_ADDRESS *src, BACNET_NPDU_DATA *npdu_data) { return bacnet_npdu_decode(npdu, MAX_NPDU, dest, src, npdu_data); } I'm assuming this is for compatibility, but wanted to point out the unsafe code path.
Out of bounds accesses in bacnet_npdu_decode
This was meant to be placed in v1.1.1 !
Out of bounds jump in h_apdu.c:apdu_handler