Bug 2069

Summary: Integer overflow in ByteTagList
Product: ns-3 Reporter: xnuvtv
Component: networkAssignee: ns-bugs <ns-bugs>
Status: RESOLVED FIXED    
Severity: normal CC: l.salameh, tomh
Priority: P5    
Version: ns-3-dev   
Hardware: All   
OS: All   
See Also: https://www.nsnam.org/bugzilla/show_bug.cgi?id=2308
Attachments: Patch that fixes m_used variable type.

Description xnuvtv 2015-02-19 17:19:54 UTC 
The m_used field in ByteTagList, which represents the number of used bytes, is 16 bit long, while all other related variables, such as offsets, sizes of tags etc. are 32-bit long.

As a result, when the number of byte tags is large, m_used variable overflows and either some assert is triggered or segmentation fault happens eventually.

An example of this bug triggered can be seen in the message sent to ns-3-users mailing list more than one year ago:
https://groups.google.com/forum/#!topic/ns-3-users/pfBiSgss-Y4

As can be seen appendOffset in ByteTagList::AddAtEnd call is 63248. As offset nearly hit the limit of 65536, m_used overflows after appending byte tag.

The solution is simple: change type of m_used variable to uint32_t. It fixed the problem in my case.
Comment 1 xnuvtv 2015-02-22 10:44:56 UTC 
Created attachment 1972 [details]
Patch that fixes m_used variable type.
Comment 2 Tom Henderson 2015-02-24 13:00:48 UTC 
We can make this change, but before we do so, I am curious how this is arising in practice.  What circumstances of tag use cause m_used to grow beyond uint16_t size?

I observed that in our current test suite, m_used never exceeds 5000.  We ought to understand the use cases that lead to this, and whether there needs to be some benchmarking of the tag search procedure, or a test put in place for packets with large tag buffers.
Comment 3 Tom Henderson 2016-09-01 21:09:17 UTC 
Regarding my question about what triggers this, we have seen accumulation of byte tags in bug 2308.

I pushed the fix in commit 12287:8e753a484600
Comment 4 l.salameh 2016-10-04 10:55:34 UTC 
*** Bug 2515 has been marked as a duplicate of this bug. ***