openigtlink / OpenIGTLink

Free, open-source network communication library for image-guided therapy
http://openigtlink.org
BSD 3-Clause "New" or "Revised" License
102 stars 184 forks source link

Indexing of type-array multiplies sizeof(T) twice #183

Closed eldruin closed 6 years ago

eldruin commented 6 years ago

I noticed that the size of the type in the Array<T> type is taken twice into account. Previously this code used bytewise indexing, which made necessary multiplying the input index with the size of the type: https://github.com/openigtlink/OpenIGTLink/blob/5acf3b41a91160b3bb875fd3bb66ae87596e4ea2/Source/igtlNDArrayMessage.h#L94 However, after casting the raw array to T* in the ByteArray variable, indexing already takes into account the size of the type. The current code would produce out-of-bounds access for types with size > 1 byte and input indices > 1.

I will submit a PR fixing this.

https://github.com/openigtlink/OpenIGTLink/blob/1d6cd5ce750e3b4edba340646247d1b297ea43fa/Source/igtlNDArrayMessage.h#L95

PD: Hi Longquan!! Best greetings :D

eldruin commented 6 years ago

For the record, this was fixed in https://github.com/openigtlink/OpenIGTLink/pull/184.

leochan2009 commented 6 years ago

Hi Diego,

nice to hear from you again :-) Great that you found the bug! I actually wrote a test for the NDArray message, but the test was only checking the first element in the array, so it failed to detect the bug, i will modify the test at the same time to be able to cover this bug:-)

Greetings from Boston! Longquan