[PATCH 0/2] moveconfig fixes

[PATCH 0/2] moveconfig fixes

[Markus Klotzbuecher]

Two fixes to moveconfig: the first addresses a potential security
issue reported by Heinrich Schuchardt caused by using the Python
built-in eval to expand CONFIG_ value expressions. Running moveconfig
on a maliciously prepared CONFIG could lead to execution of arbitrary
Python code. The second is a Python3 bugfix.

Markus Klotzbuecher (2):
  moveconfig: replace unsafe eval with asteval
  moveconfig: convert ps.stderr to string

 tools/moveconfig.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

-- 
2.25.0

[PATCH 1/2] moveconfig: replace unsafe eval with asteval

[Markus Klotzbuecher]

Commit b237d358b "moveconfig: expand simple expressions" added support
for expanding expressions in configs, but used the unsafe python
built-in "eval". This patch fixes this by replacing eval with the
asteval module.

Signed-off-by: Markus Klotzbuecher <mk@mkio.de>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Heiko Schocher <hs@denx.de>
Cc: Tom Rini <trini@konsulko.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Joe Hershberger <joe.hershberger@ni.com>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
---
 tools/moveconfig.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/moveconfig.py b/tools/moveconfig.py
index 36160a3977..df20ec66af 100755
--- a/tools/moveconfig.py
+++ b/tools/moveconfig.py
@@ -295,6 +295,7 @@ To see the complete list of supported options, run
 
 """
 
+import asteval
 import collections
 import copy
 import difflib
@@ -808,10 +809,11 @@ def try_expand(line):
         return line
 
     try:
+        aeval = asteval.Interpreter( usersyms=SIZES, minimal=True )
         cfg, val = re.split("=", line)
         val= val.strip('\"')
         if re.search("[*+-/]|<<|SZ_+|\(([^\)]+)\)", val):
-            newval = hex(eval(val, SIZES))
+            newval = hex(aeval(val))
             print("\tExpanded expression %s to %s" % (val, newval))
             return cfg+'='+newval
     except:
-- 
2.25.0

[PATCH 1/2] moveconfig: replace unsafe eval with asteval

[Heinrich Schuchardt]

On 2/12/20 8:46 PM, Markus Klotzbuecher wrote:
> Commit b237d358b "moveconfig: expand simple expressions" added support
> for expanding expressions in configs, but used the unsafe python
> built-in "eval". This patch fixes this by replacing eval with the
> asteval module.
>
> Signed-off-by: Markus Klotzbuecher <mk@mkio.de>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Heiko Schocher <hs@denx.de>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Joe Hershberger <joe.hershberger@ni.com>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>

Thanks for addressing this concern.

Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

[PATCH 1/2] moveconfig: replace unsafe eval with asteval

[Tom Rini]

On Wed, Feb 12, 2020 at 08:46:44PM +0100, Markus Klotzbuecher wrote:

> Commit b237d358b "moveconfig: expand simple expressions" added support
> for expanding expressions in configs, but used the unsafe python
> built-in "eval". This patch fixes this by replacing eval with the
> asteval module.
> 
> Signed-off-by: Markus Klotzbuecher <mk@mkio.de>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Heiko Schocher <hs@denx.de>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Joe Hershberger <joe.hershberger@ni.com>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200225/bc67fb60/attachment.sig>

[PATCH 2/2] moveconfig: convert ps.stderr to string

[Markus Klotzbuecher]

Printing the error message in verbose mode fails, since python3
doesn't implicitely convert bytes to strings.

Signed-off-by: Markus Klotzbuecher <mk@mkio.de>
Cc: Simon Glass <sjg@chromium.org>
Cc: Tom Rini <trini@konsulko.com>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
---
 tools/moveconfig.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/moveconfig.py b/tools/moveconfig.py
index df20ec66af..d8bf7fd071 100755
--- a/tools/moveconfig.py
+++ b/tools/moveconfig.py
@@ -1217,7 +1217,7 @@ class Slot:
                                "Failed to process.\n")
         if self.options.verbose:
             self.log += color_text(self.options.color, COLOR_LIGHT_CYAN,
-                                   self.ps.stderr.read())
+                                   self.ps.stderr.read().decode())
         self.finish(False)
 
     def do_defconfig(self):
-- 
2.25.0

[PATCH 2/2] moveconfig: convert ps.stderr to string

[Simon Glass]

On Wed, 12 Feb 2020 at 12:47, Markus Klotzbuecher <mk@mkio.de> wrote:
>
> Printing the error message in verbose mode fails, since python3
> doesn't implicitely convert bytes to strings.
>
> Signed-off-by: Markus Klotzbuecher <mk@mkio.de>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>  tools/moveconfig.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH 2/2] moveconfig: convert ps.stderr to string

[Tom Rini]

On Wed, Feb 12, 2020 at 08:46:45PM +0100, Markus Klotzbuecher wrote:

> Printing the error message in verbose mode fails, since python3
> doesn't implicitely convert bytes to strings.
> 
> Signed-off-by: Markus Klotzbuecher <mk@mkio.de>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200225/b6898bab/attachment.sig>