Fix: ports argument for overrides and notes (#755)

* Fix: Behaviour of create/modify_override/note to accept only valid port strings

* Adjust the tests

Author: y0urself

gvm/protocols/gmpv208/entities/notes.py

@@ -19,9 +19,9 @@
 
 from typing import Any, List, Optional
 
-from gvm.errors import RequiredArgument, InvalidArgumentType
+from gvm.errors import InvalidArgument, RequiredArgument, InvalidArgumentType
 from gvm.protocols.gmpv208.entities.severity import Severity, SeverityLevel
-from gvm.utils import add_filter, to_comma_list, to_bool
+from gvm.utils import add_filter, check_port, to_comma_list, to_bool
 from gvm.xml import XmlCommand
 
 
@@ -33,7 +33,7 @@ def create_note(
         *,
         days_active: Optional[int] = None,
         hosts: Optional[List[str]] = None,
-        port: Optional[int] = None,
+        port: Optional[str] = None,
         result_id: Optional[str] = None,
         severity: Optional[Severity] = None,
         task_id: Optional[str] = None,
@@ -47,7 +47,8 @@ def create_note(
             days_active: Days note will be active. -1 on
                 always, 0 off
             hosts: A list of hosts addresses
-            port: Port to which the note applies
+            port: Port to which the override applies, needs to be a string
+                  in the form {number}/{protocol}
             result_id: UUID of a result to which note applies
             severity: Severity to which note applies
             task_id: UUID of task to which note applies
@@ -78,7 +79,12 @@ def create_note(
             cmd.add_element("hosts", to_comma_list(hosts))
 
         if port:
-            cmd.add_element("port", str(port))
+            if check_port(port):
+                cmd.add_element("port", str(port))
+            else:
+                raise InvalidArgument(
+                    function=self.create_note.__name__, argument='port'
+                )
 
         if result_id:
             cmd.add_element("result", attrs={"id": result_id})
@@ -198,7 +204,7 @@ def modify_note(
         *,
         days_active: Optional[int] = None,
         hosts: Optional[List[str]] = None,
-        port: Optional[int] = None,
+        port: Optional[str] = None,
         result_id: Optional[str] = None,
         severity: Optional[Severity] = None,
         task_id: Optional[str] = None,
@@ -211,7 +217,8 @@ def modify_note(
             text: The text of the note.
             days_active: Days note will be active. -1 on always, 0 off.
             hosts: A list of hosts addresses
-            port: Port to which note applies.
+            port: Port to which the override applies, needs to be a string
+                  in the form {number}/{protocol}
             result_id: Result to which note applies.
             severity: Severity to which note applies.
             task_id: Task to which note applies.
@@ -242,7 +249,12 @@ def modify_note(
             cmd.add_element("hosts", to_comma_list(hosts))
 
         if port:
-            cmd.add_element("port", str(port))
+            if check_port(port):
+                cmd.add_element("port", str(port))
+            else:
+                raise InvalidArgument(
+                    function=self.modify_note.__name__, argument='port'
+                )
 
         if result_id:
             cmd.add_element("result", attrs={"id": result_id})

gvm/protocols/gmpv208/entities/overrides.py

@@ -18,9 +18,9 @@
 
 from typing import Any, List, Optional
 
-from gvm.errors import RequiredArgument, InvalidArgumentType
+from gvm.errors import InvalidArgument, RequiredArgument, InvalidArgumentType
 from gvm.protocols.gmpv208.entities.severity import Severity, SeverityLevel
-from gvm.utils import add_filter, to_comma_list, to_bool
+from gvm.utils import add_filter, check_port, to_comma_list, to_bool
 from gvm.xml import XmlCommand
 
 
@@ -32,7 +32,7 @@ def create_override(
         *,
         days_active: Optional[int] = None,
         hosts: Optional[List[str]] = None,
-        port: Optional[int] = None,
+        port: Optional[str] = None,
         result_id: Optional[str] = None,
         severity: Optional[Severity] = None,
         new_severity: Optional[Severity] = None,
@@ -47,7 +47,8 @@ def create_override(
             nvt_id: OID of the nvt to which override applies
             days_active: Days override will be active. -1 on always, 0 off
             hosts: A list of host addresses
-            port: Port to which the override applies
+            port: Port to which the override applies, needs to be a string
+                  in the form {number}/{protocol}
             result_id: UUID of a result to which override applies
             severity: Severity to which override applies
             new_severity: New severity for result
@@ -81,7 +82,12 @@ def create_override(
             cmd.add_element("hosts", to_comma_list(hosts))
 
         if port:
-            cmd.add_element("port", str(port))
+            if check_port(port):
+                cmd.add_element("port", str(port))
+            else:
+                raise InvalidArgument(
+                    function=self.create_override.__name__, argument='port'
+                )
 
         if result_id:
             cmd.add_element("result", attrs={"id": result_id})
@@ -215,7 +221,7 @@ def modify_override(
         *,
         days_active: Optional[int] = None,
         hosts: Optional[List[str]] = None,
-        port: Optional[int] = None,
+        port: Optional[str] = None,
         result_id: Optional[str] = None,
         severity: Optional[Severity] = None,
         new_severity: Optional[Severity] = None,
@@ -231,7 +237,8 @@ def modify_override(
             days_active: Days override will be active. -1 on always,
                 0 off.
             hosts: A list of host addresses
-            port: Port to which override applies.
+            port: Port to which the override applies, needs to be a string
+                  in the form {number}/{protocol}
             result_id: Result to which override applies.
             severity: Severity to which override applies.
             new_severity: New severity score for result.
@@ -264,7 +271,12 @@ def modify_override(
             cmd.add_element("hosts", to_comma_list(hosts))
 
         if port:
-            cmd.add_element("port", str(port))
+            if check_port(port):
+                cmd.add_element("port", str(port))
+            else:
+                raise InvalidArgument(
+                    function=self.modify_override.__name__, argument='port'
+                )
 
         if result_id:
             cmd.add_element("result", attrs={"id": result_id})

gvm/protocols/gmpv214/entities/notes.py

@@ -19,10 +19,10 @@
 
 from typing import Any, List, Optional
 
-from gvm.errors import RequiredArgument
+from gvm.errors import InvalidArgument, RequiredArgument
 from gvm.protocols.gmpv208.entities.notes import NotesMixin as Gmp208NotesMixin
 from gvm.protocols.gmpv208.entities.severity import Severity
-from gvm.utils import deprecation, to_comma_list
+from gvm.utils import check_port, deprecation, to_comma_list
 from gvm.xml import XmlCommand
 
 
@@ -48,7 +48,8 @@ def create_note(
             days_active: Days note will be active. -1 on
                 always, 0 off
             hosts: A list of hosts addresses
-            port: Port to which the note applies
+            port: Port to which the override applies, needs to be a string
+                  in the form {number}/{protocol}
             result_id: UUID of a result to which note applies
             severity: Severity to which note applies
             task_id: UUID of task to which note applies
@@ -78,7 +79,12 @@ def create_note(
             cmd.add_element("hosts", to_comma_list(hosts))
 
         if port:
-            cmd.add_element("port", str(port))
+            if check_port(port):
+                cmd.add_element("port", str(port))
+            else:
+                raise InvalidArgument(
+                    function=self.create_note.__name__, argument='port'
+                )
 
         if result_id:
             cmd.add_element("result", attrs={"id": result_id})
@@ -118,7 +124,8 @@ def modify_note(
             text: The text of the note.
             days_active: Days note will be active. -1 on always, 0 off.
             hosts: A list of hosts addresses
-            port: Port to which note applies.
+            port: Port to which the override applies, needs to be a string
+                  in the form {number}/{protocol}
             result_id: Result to which note applies.
             severity: Severity to which note applies.
             task_id: Task to which note applies.
@@ -148,7 +155,12 @@ def modify_note(
             cmd.add_element("hosts", to_comma_list(hosts))
 
         if port:
-            cmd.add_element("port", str(port))
+            if check_port(port):
+                cmd.add_element("port", str(port))
+            else:
+                raise InvalidArgument(
+                    function=self.modify_note.__name__, argument='port'
+                )
 
         if result_id:
             cmd.add_element("result", attrs={"id": result_id})

gvm/protocols/gmpv214/entities/overrides.py

@@ -19,12 +19,12 @@
 
 from typing import Any, List, Optional
 
-from gvm.errors import RequiredArgument
+from gvm.errors import InvalidArgument, RequiredArgument
 from gvm.protocols.gmpv208.entities.overrides import (
     OverridesMixin as Gmp208OverridesMixin,
 )
 from gvm.protocols.gmpv208.entities.severity import Severity
-from gvm.utils import deprecation, to_comma_list
+from gvm.utils import check_port, deprecation, to_comma_list
 from gvm.xml import XmlCommand
 
 
@@ -36,7 +36,7 @@ def create_override(
         *,
         days_active: Optional[int] = None,
         hosts: Optional[List[str]] = None,
-        port: Optional[int] = None,
+        port: Optional[str] = None,
         result_id: Optional[str] = None,
         severity: Optional[Severity] = None,
         new_severity: Optional[Severity] = None,
@@ -51,7 +51,8 @@ def create_override(
             nvt_id: OID of the nvt to which override applies
             days_active: Days override will be active. -1 on always, 0 off
             hosts: A list of host addresses
-            port: Port to which the override applies
+            port: Port to which the override applies, needs to be a string
+                  in the form {number}/{protocol}
             result_id: UUID of a result to which override applies
             severity: Severity to which override applies
             new_severity: New severity for result
@@ -83,7 +84,12 @@ def create_override(
             cmd.add_element("hosts", to_comma_list(hosts))
 
         if port:
-            cmd.add_element("port", str(port))
+            if check_port(port):
+                cmd.add_element("port", str(port))
+            else:
+                raise InvalidArgument(
+                    function=self.create_override.__name__, argument='port'
+                )
 
         if result_id:
             cmd.add_element("result", attrs={"id": result_id})
@@ -120,7 +126,7 @@ def modify_override(
         *,
         days_active: Optional[int] = None,
         hosts: Optional[List[str]] = None,
-        port: Optional[int] = None,
+        port: Optional[str] = None,
         result_id: Optional[str] = None,
         severity: Optional[Severity] = None,
         new_severity: Optional[Severity] = None,
@@ -136,7 +142,8 @@ def modify_override(
             days_active: Days override will be active. -1 on always,
                 0 off.
             hosts: A list of host addresses
-            port: Port to which override applies.
+            port: Port to which the override applies, needs to be a string
+                  in the form {number}/{protocol}
             result_id: Result to which override applies.
             severity: Severity to which override applies.
             new_severity: New severity score for result.
@@ -167,7 +174,12 @@ def modify_override(
             cmd.add_element("hosts", to_comma_list(hosts))
 
         if port:
-            cmd.add_element("port", str(port))
+            if check_port(port):
+                cmd.add_element("port", str(port))
+            else:
+                raise InvalidArgument(
+                    function=self.modify_override.__name__, argument='port'
+                )
 
         if result_id:
             cmd.add_element("result", attrs={"id": result_id})

gvm/utils.py

@@ -100,3 +100,13 @@ def add_filter(cmd, filter_string, filter_id):
 
 def is_list_like(value: Any) -> bool:
     return isinstance(value, (list, tuple))
+
+
+def check_port(value: str) -> bool:
+    try:
+        port, type = value.split('/', maxsplit=1)
+        if port.isdigit() and type:
+            return True
+        return False
+    except ValueError:
+        return False

tests/protocols/gmpv208/entities/notes/test_create_note.py

@@ -18,7 +18,7 @@
 
 from decimal import Decimal
 
-from gvm.errors import RequiredArgument, InvalidArgumentType
+from gvm.errors import InvalidArgument, RequiredArgument, InvalidArgumentType
 
 from gvm.protocols.gmpv208 import SeverityLevel
 
@@ -69,23 +69,13 @@ def test_create_note_with_hosts(self):
         )
 
     def test_create_note_with_port(self):
-        self.gmp.create_note('foo', nvt_oid='oid1', port='666')
+        self.gmp.create_note('foo', nvt_oid='oid1', port='666/tcp')
 
         self.connection.send.has_been_called_with(
             '<create_note>'
             '<text>foo</text>'
             '<nvt oid="oid1"/>'
-            '<port>666</port>'
-            '</create_note>'
-        )
-
-        self.gmp.create_note('foo', nvt_oid='oid1', port=666)
-
-        self.connection.send.has_been_called_with(
-            '<create_note>'
-            '<text>foo</text>'
-            '<nvt oid="oid1"/>'
-            '<port>666</port>'
+            '<port>666/tcp</port>'
             '</create_note>'
         )
 
@@ -190,3 +180,10 @@ def test_create_note_with_days_active(self):
             '<active>3600</active>'
             '</create_note>'
         )
+
+    def test_create_note_with_invalid_port(self):
+        with self.assertRaises(InvalidArgument):
+            self.gmp.create_note(text='foo', nvt_oid='oid1', port='123')
+
+        with self.assertRaises(InvalidArgument):
+            self.gmp.create_note(text='foo', nvt_oid='oid1', port='tcp/123')