Это все еще неконтролируемая командная строка, в которой может выполняться вредоносный код?
import os
command = "conda run -n python3.5 python generate_handwriting.py -text '{}' -style {} -bias {} -stroke_color '{}' -stroke_width {} -output '{}'".format(text, style, bias, stroke_color, stroke_width, output_filename)
os.system(command)
Переменные, например text, я получаю непосредственно от пользователя. Меня уведомили, что это может позволить пользователю выполнить вредоносный код. Я переписал код так:
import subprocess
cmd = ["conda", "run", "-n", "python3.5", "python", "generate_handwriting.py"]
args = ["-text", str(text), "-style", str(style), "-bias", str(bias), "-stroke_color", str(stroke_color), "-stroke_width", str(stroke_width), "-output", output_filename]
process = subprocess.run(cmd + args, check=True)
Исправлена ли проблема сейчас?
Полный код можно найти здесь.
Чтобы сделать это безопасным, вам нужно будет проверить значения каждой переменной, например, что stroke_color было шестнадцатеричной цветовой строкой (?) и ничем больше. Если какая-либо из переменных не имеет ограниченного набора допустимых значений, это будет невозможно (например, если text var может быть «любым текстом»... это затрудняет ее защиту)
@Anentropic Не уверен, что это уже редактировалось, но в нынешнем виде вы ошибаетесь. subprocess.run ограничивает выполнение одной командой и по умолчанию запрещает перенаправления. Это также позволяет использовать форму списка, где каждый элемент списка соответствует ровно одному аргументу, поэтому вызов, скорее всего, просто завершится неудачно из-за вредоносного ввода. Однако есть и другие потенциальные проблемы безопасности.
Ответы 1
Инъекция оболочки
Команда subprocess.run , в отличие от os.system, по умолчанию не допускает произвольного выполнения оболочки, например перенаправления или нескольких команд в одной строке. Для этого вам придется явно установить shell=True.
Кроме того, вы помещаете аргументы в список, который во многом является эквивалентом подготовленных операторов SQL в Python.
Поэтому гораздо лучше сделать это так.
Тем не менее, я бы все равно посоветовал вам избежать ввода данных пользователем с помощью shlex.quote, просто из общего принципа. Например, представьте, что вы должны передать для запуска одну строковую команду вместо списка команд, тогда злонамеренный пользователь может передать что-то вроде -c "my arbitrary command".
В вашем конкретном случае это не должно быть проблемой, но лучше перестраховаться, чем потом сожалеть.
Другие вопросы
Если этот процесс выполняется с более высокими правами, чем у пользователя, вам необходимо также закрыть другие пути атаки. Некоторые очевидные из них:
- Проверка наличия у пользователя разрешений на редактирование места назначения вывода.
- Ограничиваем доступ для записи на
generate_handwriting.py. - Отравление PATH, когда исполняемый файл
condaпредоставляется пользователем.
Но это мои мысли, и их вполне может быть больше.
Я не думаю, что приведенное выше изменение имеет какое-либо значение для возможности внедрения кода, оба варианта функционально одинаковы.