Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Popen([..], shell=True, [..]) can be avoided and may be exploitable (command injection) #2

Closed
hartwork opened this issue Feb 12, 2023 · 2 comments

Comments

@hartwork
Copy link

Hi!

I found this code:

pypeek/src/pypeek/main.py

Lines 1049 to 1058 in ac94d6a

systemcall = str(self.ffmpeg_bin)+" -r " + str(self.true_fps) + " -y"
systemcall += " -start_number " + str(start_number)
systemcall += " -i " + str(fprefix)+"%"+str(self.fmt)+".jpg"
systemcall += " -vframes " + str(vframes)
systemcall += " "+self.ffmpeg_flags[self.v_ext + self.quality]
systemcall += " "+str(vidfile)
systemcall += " -progress pipe:1"
try:
process = subprocess.Popen(systemcall, shell=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, encoding='utf-8', errors='replace')

If any of the variables going systemcall can be controlled by an attacker, then this is a command injection vulnerability.
Either way, I would resolve needless(?) shell=True here and build a list to call that command, not a flat string.

Thanks, Sebastian

@firatkiral
Copy link
Owner

@hartwork thank you, looking...

@hartwork
Copy link
Author

@firatkiral found 870cd31 about it now, very nice, thank you! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants