scroot / gopacket

Automatically exported from code.google.com/p/gopacket
BSD 3-Clause "New" or "Revised" License
0 stars 1 forks source link

IPv4 flags are encoded and decoded incorrectly #29

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
In https://code.google.com/p/gopacket/source/browse/layers/ip4.go, you have the 
code:

const (
        IPv4EvilBit       IPv4Flag = 1 << 0 // http://tools.ietf.org/html/rfc3514 ;)
        IPv4DontFragment  IPv4Flag = 1 << 1
        IPv4MoreFragments IPv4Flag = 1 << 2
)

func (ip *IPv4) SerializeTo(b gopacket.SerializeBuffer, opts 
gopacket.SerializeOptions) error {
        ...
        binary.BigEndian.PutUint16(bytes[6:], ip.flagsfrags())
        ...
}

func (ip *IPv4) flagsfrags() (ff uint16) {
        ff |= uint16(ip.Flags) << 13
        ff |= ip.FragOffset
        return
}

Unfortunately, this is wrong. The shift <<13 is correct, but your order of the 
flags is wrong. As http://en.wikipedia.org/wiki/IPv4#Header says:
"Flags 
A three-bit field follows and is used to control or identify fragments. They 
are (in order, from high order to low order):
bit 0: Reserved; must be zero.[note 1]
bit 1: Don't Fragment (DF)
bit 2: More Fragments (MF)"

The crucial bit is "high to low". I.e. bit 0 is actually the top bit in byte 6 
of the header. Thus you serialise MoreFrags to the top bit, when it should be 
the lowest bit. If you try to generate a packet and then decode it in wireshark 
or equiv with MoreFrags set, it will show you you've actually set the EvilBit.

The DecodeFromBytes is also wrong in the same way.

Original issue reported on code.google.com by matthew....@gmail.com on 25 Jul 2014 at 2:34

GoogleCodeExporter commented 9 years ago
Fixed.

This actually found another bug, where we incorrectly handled fragmented 
packets by treating every fragment as the top-level fragment.  See 
https://code.google.com/p/gopacket/source/detail?r=42292914602c97d8cfdf766539c2d
6726b9a0bb2 for more details.

Original comment by gconnell@google.com on 28 Jul 2014 at 5:02