From 881980d3999c698e52cbe1299ca2e63dd3838170 Mon Sep 17 00:00:00 2001
From: Guillaume Valadon <guillaume.valadon@ssi.gouv.fr>
Date: Tue, 27 Jun 2017 13:08:54 +0200
Subject: [PATCH] Catch OSError exceptions from subprocess.Popen()

---
 scapy/packet.py |  9 ++++--
 scapy/plist.py  |  6 ++--
 scapy/utils.py  | 76 +++++++++++++++++++++++++++++++++----------------
 3 files changed, 62 insertions(+), 29 deletions(-)

diff --git a/scapy/packet.py b/scapy/packet.py
index 77f75c76..ac8dd4dd 100644
--- a/scapy/packet.py
+++ b/scapy/packet.py
@@ -19,7 +19,8 @@ from scapy.fields import StrField, ConditionalField, Emph, PacketListField, BitF
 from scapy.config import conf
 from scapy.base_classes import BasePacket, Gen, SetGen, Packet_metaclass
 from scapy.volatile import VolatileValue
-from scapy.utils import import_hexcap,tex_escape,colgen,get_temp_file
+from scapy.utils import import_hexcap,tex_escape,colgen,get_temp_file, \
+    ContextManagerSubprocess
 from scapy.error import Scapy_Exception, log_runtime
 from scapy.consts import PYX
 import scapy.modules.six as six
@@ -472,7 +473,8 @@ class Packet(six.with_metaclass(Packet_metaclass, BasePacket)):
         if filename is None:
             fname = get_temp_file(autoext=".eps")
             canvas.writeEPSfile(fname)
-            subprocess.Popen([conf.prog.psreader, fname+".eps"])
+            with ContextManagerSubprocess("psdump()"):
+                subprocess.Popen([conf.prog.psreader, fname+".eps"])
         else:
             canvas.writeEPSfile(filename)
 
@@ -489,7 +491,8 @@ class Packet(six.with_metaclass(Packet_metaclass, BasePacket)):
         if filename is None:
             fname = get_temp_file(autoext=".pdf")
             canvas.writePDFfile(fname)
-            subprocess.Popen([conf.prog.pdfreader, fname+".pdf"])
+            with ContextManagerSubprocess("pdfdump()"):
+                subprocess.Popen([conf.prog.pdfreader, fname+".pdf"])
         else:
             canvas.writePDFfile(filename)
 
diff --git a/scapy/plist.py b/scapy/plist.py
index e476756d..a066f490 100644
--- a/scapy/plist.py
+++ b/scapy/plist.py
@@ -432,7 +432,8 @@ lfilter: truth function to apply to each packet to decide whether it will be dis
         if filename is None:
             filename = get_temp_file(autoext=".ps")
             d.writePSfile(filename)
-            subprocess.Popen([conf.prog.psreader, filename+".ps"])
+            with ContextManagerSubprocess("psdump()"):
+                subprocess.Popen([conf.prog.psreader, filename+".ps"])
         else:
             d.writePSfile(filename)
         print()
@@ -445,7 +446,8 @@ lfilter: truth function to apply to each packet to decide whether it will be dis
         if filename is None:
             filename = get_temp_file(autoext=".pdf")
             d.writePDFfile(filename)
-            subprocess.Popen([conf.prog.pdfreader, filename+".pdf"])
+            with ContextManagerSubprocess("psdump()"):
+                subprocess.Popen([conf.prog.pdfreader, filename+".pdf"])
         else:
             d.writePDFfile(filename)
         print()
diff --git a/scapy/utils.py b/scapy/utils.py
index 6def3584..efa3698b 100644
--- a/scapy/utils.py
+++ b/scapy/utils.py
@@ -407,6 +407,27 @@ def ltoa(x):
 def itom(x):
     return (0xffffffff00000000>>x)&0xffffffff
 
+class ContextManagerSubprocess(object):
+    """
+    Context manager that eases checking for unknown command.
+
+    Example:
+    >>> with ContextManagerSubprocess("my custom message"):
+    >>>     subprocess.Popen(["unknown_command"])
+
+    """
+    def __init__(self, name):
+        self.name = name
+
+    def __enter__(self):
+	pass
+
+    def __exit__(self, exc_type, exc_value, traceback):
+        if exc_type == OSError:
+            msg = "%s: executing %r failed"
+            log_scapy.error(msg, self.name, conf.prog.wireshark, exc_info=1)
+            return True  # Suppress the exception
+
 def do_graph(graph,prog=None,format=None,target=None,type=None,string=None,options=None):
     """do_graph(graph, prog=conf.prog.dot, format="svg",
          target="| conf.prog.display", options=None, [string=1]):
@@ -456,7 +477,8 @@ def do_graph(graph,prog=None,format=None,target=None,type=None,string=None,optio
             if conf.prog.display == conf.prog._default:
                 os.startfile(tempfile)
             else:
-                subprocess.Popen([conf.prog.display, tempfile])
+                with ContextManagerSubprocess("do_graph()"):
+                    subprocess.Popen([conf.prog.display, tempfile])
 
 _TEX_TR = {
     "{":"{\\tt\\char123}",
@@ -1127,7 +1149,8 @@ def wireshark(pktlist):
     """Run wireshark on a list of packets"""
     f = get_temp_file()
     wrpcap(f, pktlist)
-    subprocess.Popen([conf.prog.wireshark, "-r", f])
+    with ContextManagerSubprocess("wireshark()"):
+        subprocess.Popen([conf.prog.wireshark, "-r", f])
 
 @conf.commands.register
 def tcpdump(pktlist, dump=False, getfd=False, args=None,
@@ -1185,17 +1208,19 @@ u'64'
     elif isinstance(prog, six.string_types):
         prog = [prog]
     if pktlist is None:
-        proc = subprocess.Popen(
-            prog + (args if args is not None else []),
-            stdout=subprocess.PIPE if dump or getfd else None,
-            stderr=open(os.devnull),
-        )
+        with ContextManagerSubprocess("tcpdump()"):
+            proc = subprocess.Popen(
+                prog + (args if args is not None else []),
+                stdout=subprocess.PIPE if dump or getfd else None,
+                stderr=open(os.devnull),
+            )
     elif isinstance(pktlist, six.string_types):
-        proc = subprocess.Popen(
-            prog + ["-r", pktlist] + (args if args is not None else []),
-            stdout=subprocess.PIPE if dump or getfd else None,
-            stderr=open(os.devnull),
-        )
+        with ContextManagerSubprocess("tcpdump()"):
+            proc = subprocess.Popen(
+                prog + ["-r", pktlist] + (args if args is not None else []),
+                stdout=subprocess.PIPE if dump or getfd else None,
+                stderr=open(os.devnull),
+            )
     elif DARWIN:
         # Tcpdump cannot read from stdin, see
         # <http://apple.stackexchange.com/questions/152682/>
@@ -1206,19 +1231,21 @@ u'64'
             wrpcap(tmpfile, pktlist)
         else:
             tmpfile.close()
-        proc = subprocess.Popen(
-            prog + ["-r", tmpfile.name] + (args if args is not None else []),
-            stdout=subprocess.PIPE if dump or getfd else None,
-            stderr=open(os.devnull),
-        )
+        with ContextManagerSubprocess("tcpdump()"):
+            proc = subprocess.Popen(
+                prog + ["-r", tmpfile.name] + (args if args is not None else []),
+                stdout=subprocess.PIPE if dump or getfd else None,
+                stderr=open(os.devnull),
+            )
         conf.temp_files.append(tmpfile.name)
     else:
-        proc = subprocess.Popen(
-            prog + ["-r", "-"] + (args if args is not None else []),
-            stdin=subprocess.PIPE,
-            stdout=subprocess.PIPE if dump or getfd else None,
-            stderr=open(os.devnull),
-        )
+        with ContextManagerSubprocess("tcpdump()"):
+            proc = subprocess.Popen(
+                prog + ["-r", "-"] + (args if args is not None else []),
+                stdin=subprocess.PIPE,
+                stdout=subprocess.PIPE if dump or getfd else None,
+                stderr=open(os.devnull),
+            )
         try:
             proc.stdin.writelines(iter(lambda: pktlist.read(1048576), ""))
         except AttributeError:
@@ -1236,7 +1263,8 @@ def hexedit(x):
     x = str(x)
     f = get_temp_file()
     open(f,"w").write(x)
-    subprocess.call([conf.prog.hexedit, f])
+    with ContextManagerSubprocess("hexedit()"):
+        subprocess.call([conf.prog.hexedit, f])
     x = open(f).read()
     os.unlink(f)
     return x
-- 
GitLab