7

I am using Process.Start in a Windows Service I created to execute an exe that I have on my server that is used for audio conversion. I am passing some user input as a parameter to this exe. The code looks something like this:

string filePath = "\ffmpeg.exe";
string parameters =  String.Format(@"-i ""{0}"" -f mp3 ""{1}""", 
                           LocalFileName,
                           tempDirectory + NewFileName);
ProcessStartInfo startInfo = new ProcessStartInfo(ffmpegPath, parameters);
Process proc = Process.Start(startInfo);

The variables LocalFileName and NewFileName are set from user input. If I want to protect against OS Injection, is it sufficient to strip out any & characters or is there more that I should be doing?

Abe Miessler
  • 8,195
  • 11
  • 49
  • 73
  • How is an attack possible if you are giving only filenames as parameters? I don't think you should validate c# against problems similar to sql injection – techno May 09 '14 at 03:11
  • 1
    My concern is that a file could be named something like: test & timeout 1000 – Abe Miessler May 09 '14 at 05:21
  • But you are the owner of the process and you decide what the process does.How can a process execute command lines passed to it as filename parameters – techno May 09 '14 at 06:51
  • Using & as I mentioned above... Are you saying this isn't possible? Just to clarify - you do know what & does in the Windows command prompt correct? – Abe Miessler May 09 '14 at 15:27
  • well what does it do? :),want to see your view – techno May 09 '14 at 15:46
  • It allows you to execute multiple commands in a single line. So if you entered cd c:\ & dir it would execute both of those commands. As you can imagine, allowing a malicious user to do this could cause serious problems. – Abe Miessler May 09 '14 at 15:49
  • Process.start() is used to run a single File.And you are not passing the command to it from the user,you are only passing the string filenames as parameters. ie:you don't ask the user for the command. All he needs to provide is the filenames,i dont see a possibility of using & there. – techno May 09 '14 at 15:53
  • What you should focus on is the ffmpeg exploits,like DOS by overloading conversion jobs – techno May 09 '14 at 15:54
  • Interesting, so .NET automatically handles potentially malicious input that is in the parameters? – Abe Miessler May 09 '14 at 16:33

2 Answers2

4

Extra arguments into the parameters Strip any quotes from your string to ensure they can't throw in there own arguments into the command-line.

Eat your disk space Strip out ./\ characters. As if you don't you'll get an attack like "\Windows\Filename" would actually write file to ":\Windows\TempDirName\Filename" and when you later need to clean your temp files the attackers file will be in different location than you'll be looking to clean.

E.g. You'll be looking at "C:\AudioConversion\Temp\" When attackers file will be at: "C:\Windows\Temp\"

Does ffmpeg.exe have any vulnerabilities in version your using? Have a look at penetration websites for any vulnerabilities against this tool. If you're familiar with this process ofcourse give it ago yourself for extra safety. You could enable DEP for this process if you're paranoid.

Buffer overflows in your program? Yes, highly unlikely unless you're using unsafe modifier or making native calls etc but do review it.

Paul
  • 1,552
  • 11
  • 11
1

You're starting the process with parameters, not submitting a command line. Using Process.Start is like running parameterized SQL.

N Jones
  • 111
  • 2
  • 1
    He should also make sure the closing quotes cannot be escaped ("{0}" by a ") in the LocalFileName variable. I'm not sure if this refers to a valid file-system file or a user set value though. – SilverlightFox May 09 '14 at 08:56