Avoid using unvalidated strings in eval

[DanShort12]

I've noticed that the density equations are evaluated by using just eval(self.density_equation). Strings passed into eval should really be validated - particularly since the material.py module imports os, so in principle any os command could be executed. For example, if I create a json file containing the following I see that creating BadMat with some temperature in K will run the os.system command that I have provided.

{
    "BadMat": {
        "chemical_equation": "H2O",
        "density_equation": "os.system('ls')",
        "density_unit": "g/cm3",
        "temperature_dependant": true,
        "percent_type": "ao"
    }
}

There are a few tools for handling this, such as asteval, or numexpr. Note that this may need the string to be parsed to replace the temperature (or other material attributes) in the density_equation.

[makeclean]

or perhaps rm -rf /

…

On Wed, 2020-10-07 at 07:26 -0700, DanShort12 wrote: I've noticed that the density equations are evaluated by using just eval(self.density_equation). Strings passed into eval should really be validated - particularly since the material.py module imports os, so in principle any os command could be executed. For example, if I create a json file containing the following I see that creating BadMat with some temperature in K will run the os.system command that I have provided. { "BadMat": { "chemical_equation": "H2O", "density_equation": "os.system('ls')", "density_unit": "g/cm3", "temperature_dependant": true, "percent_type": "ao" } } There are a few tools for handling this, such as asteval, or numexpr. Note that this may need the string to be parsed to replace the temperature (or other material attributes) in the density_equation. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

[DanShort12]

Indeed, I picked something less exciting for the example just in case someone decided to actually run it :) But the point goes that this essentially lets anything get executed (perhaps not so bad if just running on a local machine, but wouldn't be great if this package was used to load arbitrary materials on a distributed system, for example).

[shimwell]

Thanks for the fix