lanl / Pavilion

HPC testing harness
BSD 3-Clause "New" or "Revised" License
16 stars 12 forks source link

"shell=True" in subprocess call + env variables is susceptible to shell injection. #31

Open BenCasses opened 7 years ago

BenCasses commented 7 years ago

for example, need to change all of these: output = subprocess.check_output("scontrol show hostnames \"$SLURM_NODELIST\" | tr '\n' ','", shell=True) to these: subprocess.check_output(["scontrol","show","hostnames",os.environ["SLURM_NODELIST"]]).replace("\n"," ")[:-1] In this specific case, the quotes around $SLURM_NODELIST appear to condition adequately, but I think in general it is unsafe to use "shell=True" when unncessary.

cadejager commented 7 years ago

Great, are you making these changes or should I

BenCasses commented 7 years ago

I'll get them.