From ef4f3d2f9e5629c66b02bd376f6f8e62dfaffb63 Mon Sep 17 00:00:00 2001 From: Corran Webster Date: Fri, 17 Sep 2021 10:38:24 +0100 Subject: [PATCH 1/3] Replace eval() with ast.literal_eval() We don't need the full power of eval() here, literal_eval() should do everything we need, is safer, and anything which breaks can be modified to use a custom trait that is more focused. --- apptools/preferences/preference_binding.py | 6 ++++-- apptools/preferences/preferences_helper.py | 6 ++++-- apptools/preferences/tests/example.ini | 1 + .../tests/test_preference_binding.py | 19 ++++++++++++++++++- .../tests/test_preferences_helper.py | 17 ++++++++++++++++- 5 files changed, 43 insertions(+), 6 deletions(-) diff --git a/apptools/preferences/preference_binding.py b/apptools/preferences/preference_binding.py index 0bb689814..7d8f57618 100644 --- a/apptools/preferences/preference_binding.py +++ b/apptools/preferences/preference_binding.py @@ -9,6 +9,7 @@ # Thanks for using Enthought open source! """ A binding between a trait on an object and a preference value. """ +from ast import literal_eval # Enthought library imports. from traits.api import Any, HasTraits, Instance, Str, Undefined @@ -101,10 +102,11 @@ def _get_value(self, trait_name, value): if type(handler) is Str: pass - # Otherwise, we eval it! + # Otherwise, we literal_eval it! This is safe against arbitrary code + # execution, but it does limit values to core Python data types. else: try: - value = eval(value) + value = literal_eval(value) # If the eval fails then there is probably a syntax error, but # we will let the handler validation throw the exception. diff --git a/apptools/preferences/preferences_helper.py b/apptools/preferences/preferences_helper.py index cc9201c87..b5afe0bd5 100644 --- a/apptools/preferences/preferences_helper.py +++ b/apptools/preferences/preferences_helper.py @@ -11,6 +11,7 @@ # Standard library imports. +from ast import literal_eval import logging # Enthought library imports. @@ -154,10 +155,11 @@ def _get_value(self, trait_name, value): if isinstance(handler, Str) or trait.is_str: pass - # Otherwise, we eval it! + # Otherwise, we literal_eval it! This is safe against arbitrary code + # execution, but it does limit values to core Python data types. else: try: - value = eval(value) + value = literal_eval(value) # If the eval fails then there is probably a syntax error, but # we will let the handler validation throw the exception. diff --git a/apptools/preferences/tests/example.ini b/apptools/preferences/tests/example.ini index 100dc6e16..4e9a82402 100644 --- a/apptools/preferences/tests/example.ini +++ b/apptools/preferences/tests/example.ini @@ -6,6 +6,7 @@ visible = True description = 'acme ui' offsets = "[1, 2, 3, 4]" names = "['joe', 'fred', 'jane']" +invalid = "sum(range(100))" [acme.ui.splash_screen] image = splash diff --git a/apptools/preferences/tests/test_preference_binding.py b/apptools/preferences/tests/test_preference_binding.py index 4720b05b2..2b67d0907 100644 --- a/apptools/preferences/tests/test_preference_binding.py +++ b/apptools/preferences/tests/test_preference_binding.py @@ -26,7 +26,7 @@ from apptools.preferences.api import Preferences from apptools.preferences.api import bind_preference from apptools.preferences.api import set_default_preferences -from traits.api import Bool, HasTraits, Int, Float, Str +from traits.api import Bool, HasTraits, Int, Float, Str, TraitError from traits.observation.api import match @@ -321,3 +321,20 @@ class AcmeUI(HasTraits): self.assertEqual("color", listener.trait_name) self.assertEqual("blue", listener.old) self.assertEqual("red", listener.new) + + def test_invalid_preference(self): + + p = self.preferences + p.load(self.example) + + class AcmeUI(HasTraits): + """ The Acme UI class! """ + + # The traits that we want to initialize from preferences. + invalid = Int + + acme_ui = AcmeUI() + + # Make a binding with an invalid value. + with self.assertRaises(TraitError): + bind_preference(acme_ui, "invalid", "acme.ui.invalid") diff --git a/apptools/preferences/tests/test_preferences_helper.py b/apptools/preferences/tests/test_preferences_helper.py index 207024b38..88e98d686 100644 --- a/apptools/preferences/tests/test_preferences_helper.py +++ b/apptools/preferences/tests/test_preferences_helper.py @@ -27,7 +27,7 @@ from apptools.preferences.api import ScopedPreferences from apptools.preferences.api import set_default_preferences from traits.api import ( - Any, Bool, HasTraits, Int, Float, List, Str, + Any, Bool, HasTraits, Int, Float, List, Str, TraitError, push_exception_handler, pop_exception_handler, ) from traits.observation.api import match @@ -587,3 +587,18 @@ class AcmeUIPreferencesHelper(PreferencesHelper): helper = AcmeUIPreferencesHelper(preferences_path="acme.ui") self.assertEqual("50", helper.width) + + def test_invalid_preference(self): + + p = self.preferences + p.load(self.example) + + class AcmeUIPreferencesHelper(PreferencesHelper): + """ The Acme UI class! """ + + # The traits that we want to initialize from preferences. + invalid = Int + + # attempt to create instance from invalid value + with self.assertRaises(TraitError): + helper = AcmeUIPreferencesHelper(preferences_path="acme.ui") From a06b6f5db9bf310c79f94b02f52ca06efd8ec0b7 Mon Sep 17 00:00:00 2001 From: Corran Webster Date: Fri, 17 Sep 2021 10:49:29 +0100 Subject: [PATCH 2/3] Add snippet. --- docs/releases/upcoming/299.bugfix.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 docs/releases/upcoming/299.bugfix.rst diff --git a/docs/releases/upcoming/299.bugfix.rst b/docs/releases/upcoming/299.bugfix.rst new file mode 100644 index 000000000..d2b7229ef --- /dev/null +++ b/docs/releases/upcoming/299.bugfix.rst @@ -0,0 +1 @@ +Replace eval with ast.literal_eval in apptools.preferences (#299) \ No newline at end of file From fe1d4fca072b43fe12e63980e0e7e8890b30a64c Mon Sep 17 00:00:00 2001 From: Corran Webster Date: Fri, 17 Sep 2021 10:52:25 +0100 Subject: [PATCH 3/3] Fix flake8 error. --- apptools/preferences/tests/test_preferences_helper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apptools/preferences/tests/test_preferences_helper.py b/apptools/preferences/tests/test_preferences_helper.py index 88e98d686..6ee616e39 100644 --- a/apptools/preferences/tests/test_preferences_helper.py +++ b/apptools/preferences/tests/test_preferences_helper.py @@ -601,4 +601,4 @@ class AcmeUIPreferencesHelper(PreferencesHelper): # attempt to create instance from invalid value with self.assertRaises(TraitError): - helper = AcmeUIPreferencesHelper(preferences_path="acme.ui") + AcmeUIPreferencesHelper(preferences_path="acme.ui")