CVE-2024-23346: Insufficient Information (NVD-CWE-noinfo)
value: Pymatgen (Python Materials Genomics) is an open-source Python library for materials analysis. A critical security vulnerability exists in the `JonesFaithfulTransformation.from_transformation_str()` method within the `pymatgen` library prior to version 2024.2.20. This method insecurely utilizes `eval()` for processing input, enabling execution of arbitrary code when parsing untrusted input. Version 2024.2.20 fixes this issue.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-02-21T17:15Z
Repository: https://github.com/materialsproject/pymatgen
Language: Python
Files changed: pymatgen/symmetry/settings.py
Commit date: 2024-02-11 18:03:53 -0800
Commit message: Update `JonesFaithfulTransformation` to use sympy

Signed-off-by: Matthew Horton <mkhorton@users.noreply.github.com>
Diff: diff --git a/pymatgen/symmetry/settings.py b/pymatgen/symmetry/settings.py
index a0d305098..87d4fde6f 100644
--- a/pymatgen/symmetry/settings.py
+++ b/pymatgen/symmetry/settings.py
@@ -4,10 +4,12 @@ from __future__ import annotations
 
 import re
 from fractions import Fraction
 
 import numpy as np
+from sympy import Matrix
+from sympy.parsing.sympy_parser import parse_expr
 
 from pymatgen.core.lattice import Lattice
 from pymatgen.core.operations import MagSymmOp, SymmOp
 from pymatgen.util.string import transformation_to_string
 
@@ -97,25 +99,33 @@ class JonesFaithfulTransformation:
         try:
             a, b, c = np.eye(3)
             b_change, o_shift = transformation_string.split(";")
             basis_change = b_change.split(",")
             origin_shift = o_shift.split(",")
+
             # add implicit multiplication symbols
             basis_change = [
                 re.sub(r"(?<=\w|\))(?=\() | (?<=\))(?=\w) | (?<=(\d|a|b|c))(?=([abc]))", r"*", string, flags=re.X)
                 for string in basis_change
             ]
-            # should be fine to use eval here but be mindful for security
-            # reasons
-            # see http://lybniz2.sourceforge.net/safeeval.html
-            # could replace with regex? or sympy expression?
-            P = np.array([eval(x, {"__builtins__": None}, {"a": a, "b": b, "c": c}) for x in basis_change])
-            P = P.transpose()  # by convention
+
+            # basic input sanitation
+            allowed_chars = "0123456789+-*/.abc()"
+            basis_change = ["".join([c for c in string if c in allowed_chars]) for string in basis_change]
+
+            # requires round-trip to sympy to evaluate
+            # (alternatively, `numexpr` looks like a nice solution but requires an additional dependency)
+            basis_change = [
+                parse_expr(string).subs({"a": Matrix(a), "b": Matrix(b), "c": Matrix(c)}) for string in basis_change
+            ]
+            # convert back to numpy, perform transpose by convention
+            P = np.array(basis_change, dtype=float).T[0]
+
             p = [float(Fraction(x)) for x in origin_shift]
             return P, p
-        except Exception:
-            raise ValueError("Failed to parse transformation string.")
+        except Exception as exc:
+            raise ValueError(f"Failed to parse transformation string: {exc}")
 
     @property
     def P(self) -> list[list[float]]:
         """Transformation matrix."""
         return self._P

Vulnerable code:             # should be fine to use eval here but be mindful for security
            # reasons
            # see http://lybniz2.sourceforge.net/safeeval.html
            # could replace with regex? or sympy expression?
            P = np.array([eval(x, {"__builtins__": None}, {"a": a, "b": b, "c": c}) for x in basis_change])
            P = P.transpose()  # by convention
        except Exception:
            raise ValueError("Failed to parse transformation string.")
Fixed code: from sympy import Matrix
from sympy.parsing.sympy_parser import parse_expr


            # basic input sanitation
            allowed_chars = "0123456789+-*/.abc()"
            basis_change = ["".join([c for c in string if c in allowed_chars]) for string in basis_change]

            # requires round-trip to sympy to evaluate
            # (alternatively, `numexpr` looks like a nice solution but requires an additional dependency)
            basis_change = [
                parse_expr(string).subs({"a": Matrix(a), "b": Matrix(b), "c": Matrix(c)}) for string in basis_change
            ]
            # convert back to numpy, perform transpose by convention
            P = np.array(basis_change, dtype=float).T[0]

        except Exception as exc:
            raise ValueError(f"Failed to parse transformation string: {exc}")