g_message_data_reply_buffer buffer overflow

[ccvelandres]

Just thought I'd share one of the problem I encountered.

I was having a Hard Fault on my device whenever I try the Get_Attribute_All on 0xF6 Ethernet Link with EtherNet/IP Tools from Molex. Turns out that the g_message_data_reply_buffer used by NotifyMessageRouter() was overflowing. It was sized 100 by default from the sample opener_user_conf.h on MINGW.

Increasing the buffer size fixed the hard fault problem. I think it'd be great if we knew the minimum buffer size needed.

[MartinMelikMerkumians]

Hi,

thanks your sharing. I think you can't give this minimum size in general. The absolute worst case for Ethernet Link objects Get_Attribute_All should be ‭1.259‬ bytes, if I calculated it correct.

[azoitl]

I always hoped that we could get rid of this buffer and directly user the communication buffer for that. While this would not reduce the estimation problem it would relieve from copying and double buffering.

[ccvelandres]

Maybe we could at least add an overflow checks for the buffers. Like checking the last 2 bytes if it gets written on? Though this requires for the buffer to get initialized to zero. Much preferable than getting UB from overflows

[MartinMelikMerkumians]

Good idea. I took a quick look and the global buffer is only used as part of the CipMessageRouterResponse struct, which already has a data_length field and calculated at the end of filling the message. We could use this field to track the current used length and compare it to the max length constant.

[MartinMelikMerkumians]

Hi,

a little update to this issue. The reply buffer size is now configurable via CMake, as I ran in to a similar problem when testing the Electrical Energy Object.

[MartinMelikMerkumians]

the reply message is now changed to an ENIPMessage struct, with start, length and current buffer position saved. Runtime checks need only to be added in the corresponding functions now