Repository: secdev/scapy
Pull request: Small 0 and empty string handling cases
fixes https://github.com/secdev/scapy/issues/1679
fixes https://github.com/secdev/scapy/issues/1680

Problem statement:
Setting IP length to 0 breaks bytes() packet cast
#### Brief description

If you create a packet and set the IP length to zero and try to see the bytes of the packet, scapy crashes. 
IP is affected by this. 

#### Environment

-   Scapy version: 2.4.0
-   Python version: 3.5.2
-   Operating System: Ubuntu 16.04

#### How to reproduce

```
# scapy
...
>>> p= IP()/TCP()
>>> p[IP].len = 0
>>> bytes(p)
Traceback (most recent call last):
  File "/usr/lib/python3.5/code.py", line 91, in runcode
    exec(code, self.locals)
  File "<console>", line 1, in <module>
  File "/home/ubuntu/.local/lib/python3.5/site-packages/scapy/packet.py", line 345, in __bytes__
    return self.build()
  File "/home/ubuntu/.local/lib/python3.5/site-packages/scapy/packet.py", line 444, in build
    p = self.do_build()
  File "/home/ubuntu/.local/lib/python3.5/site-packages/scapy/packet.py", line 429, in do_build
    pay = self.do_build_payload()
  File "/home/ubuntu/.local/lib/python3.5/site-packages/scapy/packet.py", line 416, in do_build_payload
    return self.payload.do_build()
  File "/home/ubuntu/.local/lib/python3.5/site-packages/scapy/packet.py", line 431, in do_build
    return self.post_build(pkt, pay)
  File "/home/ubuntu/.local/lib/python3.5/site-packages/scapy/layers/inet.py", line 528, in post_build
    ck = in4_chksum(socket.IPPROTO_TCP, self.underlayer, p)
  File "/home/ubuntu/.local/lib/python3.5/site-packages/scapy/layers/inet.py", line 504, in in4_chksum
    ln)
struct.error: argument out of range
>>> 
```
#### Actual result

Scapy throws an exception.

#### Expected result

Scapy should not throw an exception. 


Setting IP flags to '' directly raises an exception
#### Brief description

If you set IP flags directly to an empty string (should be 0, no flags), scapy raises an exception.
IP is affected by this. 

#### Environment

-   Scapy version: 2.4.0
-   Python version: 3.5.2
-   Operating System: Ubuntu 16.04


#### How to reproduce

```
>>> p[IP].flags = ""
Traceback (most recent call last):
  File "/usr/lib/python3.5/code.py", line 91, in runcode
    exec(code, self.locals)
  File "<console>", line 1, in <module>
  File "/home/ubuntu/.local/lib/python3.5/site-packages/scapy/packet.py", line 264, in __setattr__
    return self.setfieldval(attr,val)
  File "/home/ubuntu/.local/lib/python3.5/site-packages/scapy/packet.py", line 249, in setfieldval
    self.fields[attr] = any2i(self, val)
  File "/home/ubuntu/.local/lib/python3.5/site-packages/scapy/fields.py", line 1362, in any2i
    return self._fixup_val(super(FlagsField, self).any2i(pkt, x))
  File "/home/ubuntu/.local/lib/python3.5/site-packages/scapy/fields.py", line 1359, in _fixup_val
    return x if x is None or isinstance(x, FlagValue) else FlagValue(x, self.names)
  File "/home/ubuntu/.local/lib/python3.5/site-packages/scapy/fields.py", line 1246, in __init__
    self.value = self._fixvalue(value)
  File "/home/ubuntu/.local/lib/python3.5/site-packages/scapy/fields.py", line 1240, in _fixvalue
    y |= 1 << self.names.index(i)
ValueError: '' is not in list
>>> p[IP].flags = 0                    # BUT, you can set it to zero to achieve the same effect.
>>> str(p[IP].flags)
''
>>> p = IP()/TCP()
>>> p[IP].flags = 1                   # And other numerical values work
>>> str(p[IP].flags)
'MF'
>>> setattr(p[IP], "flags", "")    # Going through setattr also does not work
Traceback (most recent call last):
  File "/usr/lib/python3.5/code.py", line 91, in runcode
    exec(code, self.locals)
  File "<console>", line 1, in <module>
  File "/home/ubuntu/.local/lib/python3.5/site-packages/scapy/packet.py", line 264, in __setattr__
    return self.setfieldval(attr,val)
  File "/home/ubuntu/.local/lib/python3.5/site-packages/scapy/packet.py", line 249, in setfieldval
    self.fields[attr] = any2i(self, val)
  File "/home/ubuntu/.local/lib/python3.5/site-packages/scapy/fields.py", line 1362, in any2i
    return self._fixup_val(super(FlagsField, self).any2i(pkt, x))
  File "/home/ubuntu/.local/lib/python3.5/site-packages/scapy/fields.py", line 1359, in _fixup_val
    return x if x is None or isinstance(x, FlagValue) else FlagValue(x, self.names)
  File "/home/ubuntu/.local/lib/python3.5/site-packages/scapy/fields.py", line 1246, in __init__
    self.value = self._fixvalue(value)
  File "/home/ubuntu/.local/lib/python3.5/site-packages/scapy/fields.py", line 1240, in _fixvalue
    y |= 1 << self.names.index(i)
ValueError: '' is not in list 
```
#### Actual result

Scapy throws an exception.

#### Expected result

Scapy should not throw an exception.

Hints from issue discussion:
This behavior is expected. Flags are not strings in the real world, they are numeral. When you assign a string to a flag, you expect scapy to do name-resolution for you.

It is expected to return an error if the name resolution failed. If you wanted to set an invalid value, use the numeral assignment
The name resolution should not fail - empty string is a valid string representation of a lack of flags (or an all 0 flags field). Existing behavior already partially supports this, right?

```
>>> p[IP].flags = 0                   
>>> str(p[IP].flags)
''
```

Commit message: Small 0 and empty string handling cases

Patch to review:
diff --git a/scapy/fields.py b/scapy/fields.py
index 9af9b70d212..8b411580e09 100644
--- a/scapy/fields.py
+++ b/scapy/fields.py
@@ -1736,6 +1736,8 @@ class FlagValue(object):
     __slots__ = ["value", "names", "multi"]
 
     def _fixvalue(self, value):
+        if value is None or value == "":
+            return 0
         if isinstance(value, six.string_types):
             value = value.split('+') if self.multi else list(value)
         if isinstance(value, list):
@@ -1743,7 +1745,7 @@ def _fixvalue(self, value):
             for i in value:
                 y |= 1 << self.names.index(i)
             value = y
-        return None if value is None else int(value)
+        return int(value)
 
     def __init__(self, value, names):
         self.multi = isinstance(names, list)
diff --git a/scapy/layers/inet.py b/scapy/layers/inet.py
index cec9e601fb5..6c4cfb34eb0 100644
--- a/scapy/layers/inet.py
+++ b/scapy/layers/inet.py
@@ -541,7 +541,7 @@ def in4_chksum(proto, u, p):
             ihl = 5 + olen // 4 + (1 if olen % 4 else 0)
         else:
             ihl = u.ihl
-        ln = u.len - 4 * ihl
+        ln = max(u.len - 4 * ihl, 0)
     else:
         ln = len(p)
     psdhdr = struct.pack("!4s4sHH",
diff --git a/test/regression.uts b/test/regression.uts
index 84fb736a33c..8820bd61d79 100644
--- a/test/regression.uts
+++ b/test/regression.uts
@@ -6537,6 +6537,13 @@ pkt = IP(len=42, ihl=6, options=[IPOption_RR()]) / UDP() / ("A" * 10)
 bpkt = IP(raw(pkt))
 assert bpkt.chksum == 0x70bd and bpkt.payload.chksum == 0xbb17
 
+= IP with forced-length 0
+p = IP()/TCP()
+p[IP].len = 0
+p = IP(raw(p))
+
+assert p.len == 0
+
 = TCP payload with IP Total Length 0
 data = b'1234567890abcdef123456789ABCDEF'
 pkt = IP()/TCP()/data
@@ -9646,6 +9653,10 @@ assert repr(pkt.flags) == '<Flag 6 (DF+evil)>'
 
 assert len({IP().flags, IP().flags}) == 1
 
+pkt = IP()
+pkt.flags = ""
+assert pkt.flags == 0
+
 = TCP flags
 ~ TCP
 


Code review comments:
diff_hunk: @@ -1736,14 +1736,16 @@ class FlagValue(object):
     __slots__ = ["value", "names", "multi"]
 
     def _fixvalue(self, value):
+        if value is None or value == "":
path: scapy/fields.py
text: @user1:
```suggestion
        if not value:
```