test_shell_injection.TestShellInjection errors

[cgohlke]

I get the following test_shell_injection.TestShellInjection errors on Windows using djpeg and cjpeg executables from jpeg-9a:

test_load_djpeg_filename (test_shell_injection.TestShellInjection) ... djpeg: must name one input and one output file
usage: djpeg [switches] inputfile outputfile
Switches (names may be abbreviated):
  -colors N      Reduce image to no more than N colors
  -fast          Fast, low-quality processing
  -grayscale     Force grayscale output
  -scale M/N     Scale output image by fraction M/N, eg, 1/8
  -bmp           Select BMP output format (Windows style)
  -gif           Select GIF output format
  -os2           Select BMP output format (OS/2 style)
  -pnm           Select PBMPLUS (PPM/PGM) output format (default)
  -targa         Select Targa output format
Switches for advanced users:
  -dct int       Use integer DCT method (default)
  -dct fast      Use fast integer DCT (less accurate)
  -dct float     Use floating-point DCT method
  -dither fs     Use F-S dithering (default)
  -dither none   Don't use dithering in quantization
  -dither ordered  Use ordered dither (medium speed, quality)
  -map FILE      Map to colors used in named image file
  -nosmooth      Don't use high-quality upsampling
  -onepass       Use 1-pass quantization (fast, low quality)
  -maxmemory N   Maximum memory to use (in kbytes)
  -outfile name  Specify name for output file
  -verbose  or  -debug   Emit debug output
ERROR
test_save_cjpeg_filename (test_shell_injection.TestShellInjection) ... cjpeg: must name one input and one output file
usage: cjpeg [switches] inputfile outputfile
Switches (names may be abbreviated):
  -quality N[,...]   Compression quality (0..100; 5-95 is useful range)
  -grayscale     Create monochrome JPEG file
  -rgb           Create RGB JPEG file
  -optimize      Optimize Huffman table (smaller file, but slow compression)
  -progressive   Create progressive JPEG file
  -scale M/N     Scale image by fraction M/N, eg, 1/2
  -targa         Input file is Targa format (usually not needed)
Switches for advanced users:
  -arithmetic    Use arithmetic coding
  -block N       DCT block size (1..16; default is 8)
  -rgb1          Create RGB JPEG file with reversible color transform
  -bgycc         Create big gamut YCC JPEG file
  -dct int       Use integer DCT method (default)
  -dct fast      Use fast integer DCT (less accurate)
  -dct float     Use floating-point DCT method
  -nosmooth      Don't use high-quality downsampling
  -restart N     Set restart interval in rows, or in blocks with B
  -smooth N      Smooth dithered input (N=1..100 is strength)
  -maxmemory N   Maximum memory to use (in kbytes)
  -outfile name  Specify name for output file
  -verbose  or  -debug   Emit debug output
Switches for wizards:
  -baseline      Force baseline quantization tables
  -qtables file  Use quantization tables given in file
  -qslots N[,...]    Set component quantization tables
  -sample HxV[,...]  Set component sampling factors
  -scans file    Create multi-scan JPEG per script file
ERROR
test_save_netpbm_filename_bmp_mode (test_shell_injection.TestShellInjection) ... SKIP: netpbm not available
test_save_netpbm_filename_l_mode (test_shell_injection.TestShellInjection) ... SKIP: netpbm not available

======================================================================
ERROR: test_load_djpeg (test_file_jpeg.TestFileJpeg)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\Build\Pillow\Pillow-git\Tests\test_file_jpeg.py", line 282, in test_load_djpeg
    img.load_djpeg()
  File "D:\Build\Pillow\Pillow-git\PIL\JpegImagePlugin.py", line 364, in load_djpeg
    subprocess.check_call(["djpeg", self.filename], stdout=f)
  File "X:\Python34\lib\subprocess.py", line 561, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['djpeg', 'Tests/images/lena.jpg']' returned non-zero exit status 1

======================================================================
ERROR: test_save_cjpeg (test_file_jpeg.TestFileJpeg)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\Build\Pillow\Pillow-git\Tests\test_file_jpeg.py", line 290, in test_save_cjpeg
    JpegImagePlugin._save_cjpeg(img, 0, tempfile)
  File "D:\Build\Pillow\Pillow-git\PIL\JpegImagePlugin.py", line 610, in _save_cjpeg
    subprocess.check_call(["cjpeg", tempfile], stdout=f)
  File "X:\Python34\lib\subprocess.py", line 561, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['cjpeg', 'C:\\Users\\gohlke\\AppData\\Local\\Temp\\tmp3zfmx93y']' returned non-zero exit status 1

======================================================================
ERROR: test_load_djpeg_filename (test_shell_injection.TestShellInjection)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\Build\Pillow\Pillow-git\Tests\test_shell_injection.py", line 35, in test_load_djpeg_filename
    im.load_djpeg()
  File "D:\Build\Pillow\Pillow-git\PIL\JpegImagePlugin.py", line 364, in load_djpeg
    subprocess.check_call(["djpeg", self.filename], stdout=f)
  File "X:\Python34\lib\subprocess.py", line 561, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['djpeg', "C:\\Users\\gohlke\\AppData\\Local\\Temp\\pillow-tests\\temptests-3.4-script_121_';"]' returned non-zero exit status 1

======================================================================
ERROR: test_save_cjpeg_filename (test_shell_injection.TestShellInjection)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\Build\Pillow\Pillow-git\Tests\test_shell_injection.py", line 40, in test_save_cjpeg_filename
    self.assert_save_filename_check(im, JpegImagePlugin._save_cjpeg)
  File "D:\Build\Pillow\Pillow-git\Tests\test_shell_injection.py", line 24, in assert_save_filename_check
    save_func(src_img, 0, dest_file)
  File "D:\Build\Pillow\Pillow-git\PIL\JpegImagePlugin.py", line 610, in _save_cjpeg
    subprocess.check_call(["cjpeg", tempfile], stdout=f)
  File "X:\Python34\lib\subprocess.py", line 561, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['cjpeg', 'C:\\Users\\gohlke\\AppData\\Local\\Temp\\tmp3f3_qyhv']' returned non-zero exit status 1

[hugovk]

ping @mbrown1413 re: #731 #748

cjpeg and djpeg on Windows expect an output filename.

[mbrown1413]

I'll work on adding another case in load_cjpeg and _save_djpeg for windows this evening.

Do you know if ppmquant and ppmtogif in have the same problem?

[cgohlke]

Regarding ppmquant and ppmtogif: The netpbm tests are skipped on Windows because ppmquant is a sh script using Perl. The script will not run under Windows command interpreter (cmd.exe) without changes. ppmtogif is an executable that works on Windows.

[mbrown1413]

Pull request: #757

I don't have a Windows machine set up for development, but I think this will work. Would you mind trying my pull request in Windows? My apologizes if it doesn't work, I will set up a Windows machine soon if it doesn't.

[mbrown1413]

That would be fine for now. However, I don't think load_djpeg and _save_cjpeg should raise an OSError when double quotes or pipes are given in the filename. The filenames that the shell injection tests was intended to include many characters from many shells by design.

Do the tests in test_file_jpeg.py work for you?

[mbrown1413]

Pull request: #758

[mbrown1413]

I wasn't expecting those files to be created on Windows, I just expected a different error. I was originally thinking ValueError should be raised if an invalid filename was given, since it's usually raised when correct types, but incorrect values are given. But you're right, IOError or OSError is raised by open when invalid filenames are given, so that's what methods like load_djpeg should raise also.

It does feel odd having shell injection testing for one platform but not another, but that sounds like a problem for another day. As long as we're not using os.system anymore.

[wiredfool]

I agree -- once we're not using os.system, these tests aren't as useful.

(as an aside, these tests aren't asserting that we're safe from shell injections, they're asserting that we're safe from shell injections that we can think of. Useful for confirming known issues, not useful for asserting completeness).