Position Message aka "QTRANS" can change Content Size resulting in memory allocation errors

[lkarstensen]

After several days of trying of chasing memory allocation errors while trying to implement this OpenIGTLink Version in Python using pybind11, I realized the problem wasn't my implementation. The PositionMessage still has the functionality to change the size of its content by SetPackType, even though it is not mentioned in the protocol specification anymore.

void PositionMessage::SetPackType(int t) { if (t >= POSITION_ONLY && t <= ALL) { this->m_PackType = t; } }

But when receiving Position Messages other than Pack Type ALL the Header is not able to tell you which PackType you have since Header Version 2 due to variable extended header/meta data size. But there is still a dummy function which reads like its doing exactly that when setting the header: `int PositionMessage::SetMessageHeader(const MessageHeader* mb) { int rc = Copy(mb); int rt = SetPackTypeByContentSize(this->CalculateContentBufferSize());

return (rc && rt);
} `

I suggest removing this functionality from the Position Message Code to not confuse futuer developers using this OpenIGTLink implementation. At least this Issue Report might help them not get stuck on this problem if encoutered.

[leochan2009]

HI,

Thanks for finding the issue. i think this is a bug the line: int rt = SetPackTypeByContentSize(this->CalculateContentBufferSize()); should be int rt = SetPackTypeByContentSize(this->CalculateReceiveContentSize()); . The user should still be able to get the type from received message body when excluding the meta information.

[leochan2009]

I am testing a fix for the bug. could you test my forked branch if it works for your python binding? https://github.com/leochan2009/OpenIGTLink Thanks in advance Best, longquan

[leochan2009]

@LennyK2410 , are you able to test the forked branch for your python binding? Thanks!

[lkarstensen]

Hi,

@leochan2009 Thanks for the quick reply. Unfortunately your fix didn't solve the problem. Content Size is not known before the extended header is unpacked. I found out that setting the PackType is done in "UnpackContent", therefore the class specific SetMessageHeader is not needed. My Initial Statement with the bug happening when receiving a message was wrong. The memory error occurs when packing a message with PackType smaller than "ALL". Byte order conversion in "PackContent" is not PackType specific as in "UnpackContent". I implemented a bugfix in a forked branch, which is working for my application: https://github.com/LennyK2410/OpenIGTLink

Best Regards, Lennart

[leochan2009]

hi Lennart,

Thanks for the fix. i have overlooked the issue.

[leochan2009]

Not more feedback from user, the issue seems fixed.