Repository: hgrecco/pint

Issue:
[SECURITY] Arbitrary code execution from definitions
# Critical security vulnerability

A user who is allowed to upload a definitions file can run arbitrary code as the user running pint, thanks to two calls to eval() in definitions.py,

here
https://github.com/hgrecco/pint/blob/23c44ee335797c051feea0103e28c6a7221b3b75/pint/definitions.py#L90-L96
and here
https://github.com/hgrecco/pint/blob/23c44ee335797c051feea0103e28c6a7221b3b75/pint/definitions.py#L103-L119


Discussion:

Patch:
diff --git a/CHANGES b/CHANGES
index 19f7d7f..f1bc247 100644
--- a/CHANGES
+++ b/CHANGES
@@ -4,6 +4,8 @@ Pint Changelog
 0.10 (unreleased)
 -----------------
 
+- Removed eval usage when creating UnitDefinition and PrefixDefinition from string.
+  (Issue #942)
 - Added `fmt_locale` argument to registry.
   (Issue #904)
 - Better error message when Babel is not installed.
diff --git a/pint/definitions.py b/pint/definitions.py
index 4188124..c0481df 100644
--- a/pint/definitions.py
+++ b/pint/definitions.py
@@ -13,6 +13,20 @@ from .errors import DefinitionSyntaxError
 from .util import ParserHelper, UnitsContainer, _is_dim
 
 
+class _NotNumeric(Exception):
+    def __init__(self, value):
+        self.value = value
+
+
+def numeric_parse(s):
+    ph = ParserHelper.from_string(s)
+
+    if len(ph):
+        raise _NotNumeric(s)
+
+    return ph.scale
+
+
 class Definition:
     """Base class for definitions.
 
@@ -94,7 +108,13 @@ class PrefixDefinition(Definition):
 
     def __init__(self, name, symbol, aliases, converter):
         if isinstance(converter, str):
-            converter = ScaleConverter(eval(converter))
+            try:
+                converter = ScaleConverter(numeric_parse(converter))
+            except _NotNumeric as ex:
+                raise ValueError(
+                    f"Prefix definition ('{name}') must contain only numbers, not {ex.value}"
+                )
+
         aliases = tuple(alias.strip("-") for alias in aliases)
         if symbol:
             symbol = symbol.strip("-")
@@ -114,10 +134,19 @@ class UnitDefinition(Definition):
         if isinstance(converter, str):
             if ";" in converter:
                 [converter, modifiers] = converter.split(";", 2)
-                modifiers = dict(
-                    (key.strip(), eval(value))
-                    for key, value in (part.split(":") for part in modifiers.split(";"))
-                )
+
+                try:
+                    modifiers = dict(
+                        (key.strip(), numeric_parse(value))
+                        for key, value in (
+                            part.split(":") for part in modifiers.split(";")
+                        )
+                    )
+                except _NotNumeric as ex:
+                    raise ValueError(
+                        f"Unit definition ('{name}') must contain only numbers in modifier, not {ex.value}"
+                    )
+
             else:
                 modifiers = {}
 


Test patch:
diff --git a/pint/testsuite/test_definitions.py b/pint/testsuite/test_definitions.py
index e80622f..a5e0086 100644
--- a/pint/testsuite/test_definitions.py
+++ b/pint/testsuite/test_definitions.py
@@ -19,6 +19,9 @@ class TestDefinition(BaseTestCase):
             Definition.from_string("[x] = [time] * meter")
 
     def test_prefix_definition(self):
+
+        self.assertRaises(ValueError, Definition.from_string, "m- = 1e-3 k")
+
         for definition in ("m- = 1e-3", "m- = 10**-3", "m- = 0.001"):
             x = Definition.from_string(definition)
             self.assertIsInstance(x, PrefixDefinition)
@@ -85,6 +88,12 @@ class TestDefinition(BaseTestCase):
         self.assertEqual(x.converter.scale, 6.28)
         self.assertEqual(x.reference, UnitsContainer(radian=1))
 
+        self.assertRaises(
+            ValueError,
+            Definition.from_string,
+            "degF = 9 / 5 * kelvin; offset: 255.372222 bla",
+        )
+
     def test_dimension_definition(self):
         x = DimensionDefinition("[time]", "", (), converter="")
         self.assertTrue(x.is_base)