[U-Boot] [PATCH v2 1/9] moveconfig: expand simple expressions

[Heinrich Schuchardt]

On 1/29/20 9:44 AM, Markus Klotzbuecher wrote:
> Hi Heinrich
>
> On Sat, Jan 25, 2020 at 10:46:04PM +0100, Heinrich Schuchardt wrote:
>> On 5/15/19 3:15 PM, Markus Klotzbuecher wrote:
>>> From: Markus Klotzbuecher <markus.klotzbuecher at kistler.com>
>>>
>>> Add support for expanding simple expressions and sizes such as
>>> "(4 * 1024)", "(512 << 10)" or "(SZ_256K)".
>>>
>>> This can help to significantly reduce the number of "suspicious"
>>> moves, such as
>>>
>>>    'CONFIG_ENV_SIZE="(64 << 10)"' was removed by savedefconfig.
>>>
>>> If the expansion fails, it falls back to the original string.
>>>
>>> Signed-off-by: Markus Klotzbuecher <markus.klotzbuecher at kistler.com>
>>> Cc: Masahiro Yamada <yamada.masahiro at socionext.com>
>>> Cc: Heiko Schocher <hs at denx.de>
>>> ---
>>> Changes for v2: new patch
>>>
>>>    tools/moveconfig.py | 41 +++++++++++++++++++++++++++++++++++++++++
>>>    1 file changed, 41 insertions(+)
>>>
>>> diff --git a/tools/moveconfig.py b/tools/moveconfig.py
>>> index 1a214c5605..0bbc7c1991 100755
>>> --- a/tools/moveconfig.py
>>> +++ b/tools/moveconfig.py
>>> @@ -354,6 +354,26 @@ CONFIG_DATABASE = 'moveconfig.db'
>>>
>>>    CONFIG_LEN = len('CONFIG_')
>>>
>>> +SIZES = {
>>> +    "SZ_1":    0x00000001, "SZ_2":    0x00000002,
>>> +    "SZ_4":    0x00000004, "SZ_8":    0x00000008,
>>> +    "SZ_16":   0x00000010, "SZ_32":   0x00000020,
>>> +    "SZ_64":   0x00000040, "SZ_128":  0x00000080,
>>> +    "SZ_256":  0x00000100, "SZ_512":  0x00000200,
>>> +    "SZ_1K":   0x00000400, "SZ_2K":   0x00000800,
>>> +    "SZ_4K":   0x00001000, "SZ_8K":   0x00002000,
>>> +    "SZ_16K":  0x00004000, "SZ_32K":  0x00008000,
>>> +    "SZ_64K":  0x00010000, "SZ_128K": 0x00020000,
>>> +    "SZ_256K": 0x00040000, "SZ_512K": 0x00080000,
>>> +    "SZ_1M":   0x00100000, "SZ_2M":   0x00200000,
>>> +    "SZ_4M":   0x00400000, "SZ_8M":   0x00800000,
>>> +    "SZ_16M":  0x01000000, "SZ_32M":  0x02000000,
>>> +    "SZ_64M":  0x04000000, "SZ_128M": 0x08000000,
>>> +    "SZ_256M": 0x10000000, "SZ_512M": 0x20000000,
>>> +    "SZ_1G":   0x40000000, "SZ_2G":   0x80000000,
>>> +    "SZ_4G":  0x100000000
>>> +}
>>> +
>>>    ### helper functions ###
>>>    def get_devnull():
>>>        """Get the file object of '/dev/null' device."""
>>> @@ -777,6 +797,25 @@ def cleanup_readme(configs, options):
>>>        with open('README', 'w') as f:
>>>            f.write(''.join(newlines))
>>>
>>> +def try_expand(line):
>>> +    """If value looks like an expression, try expanding it
>>> +    Otherwise just return the existing value
>>> +    """
>>> +    if line.find('=') == -1:
>>> +        return line
>>> +
>>> +    try:
>>> +        cfg, val = re.split("=", line)
>>> +        val= val.strip('\"')
>>> +        if re.search("[*+-/]|<<|SZ_+|\(([^\)]+)\)", val):
>>> +            newval = hex(eval(val, SIZES))
>>
>> The if clause evaluates to true for values like:
>>
>>     val = "os.execl('/sbin/fdisk')"
>>
>> As eval() can be used to execute arbitrary commands this patch should be
>> corrected.
>
> Fair point. I took a quick look at python sandboxing, and apparently
> it's difficult to be done in a secure way (see pysandbox). As
> introducing a CONFIG with something like the above clearly has
> malicious intent, just preventing "accidential" execution will not be
> sufficient. Perhaps we can use ast.literal_eval instead. I'll take a
> closer look.

Except for the strings starting with SZ_ that you defined we would not
expect any letters in the term to evaluate. This could be checked using
a suitable regular expression.

Best regards

Heinrich