Securely convert and return a user selected file

I designed a Python3 webservice which gets a file name, looks for this file in a local folder, converts it using an external software, then sends the converted file to the user.

For this example, the input and output file types are respectively PNG and JPEG, but they're actually proprietary formats.

Here is my code:

``
#!/usr/bin/python3
# -- coding: utf-8 -

from http.server import BaseHTTPRequestHandler, HTTPServer
import os
import os.path as op
import subprocess as sub
from urllib.parse import parse_qs, urlparse

PORT_NUMBER = 8080
PNG_TO_JPG_CALL = '/usr/share/png-to-jpg'
PNG_FOLDER = '/home/foo/png_files'

class RequestHandler(BaseHTTPRequestHandler):
# Display a text to the user.
def display_msg(self, text):
print(text)
self.send_header('Content-type', 'text/plain; charset=utf-8')
self.send_header('Content-Disposition', 'inline')
self.end_headers()
self.wfile.write(bytes(text, 'UTF-8'))

# Send a file to the user (open the Download File dialog box).
def send_file(self, file_path, jpg_name):
self.send_header('Content-type', 'image/jpeg')
self.send_header('Content-Disposition', 'attachment; filename="%s"' % jpg_name)
self.end_headers()
with open(file_path, 'rb') as f:
self.wfile.write(f.read())

# Check for GET requests
def do_GET(self):
self.send_response(200)

# Get URL arguments
url_args = parse_qs(urlparse(self.path).query, keep_blank_values=True)

# Check for the id` attribute
if 'id' not in url_args:
self.display_msg('USAGE: [...]/?id=some_file.jpg')
return

target = url_args['id'][0]

# Check if the requested file have the good extension
if target[-4:] != '.jpg':
self.display_msg('TARGET SHOULD BE A PNG FILE')
return

# build the png file path (change the extension)
png_path = op.join(PNG_FOLDER, op.splitext(target)[

This script is insecure because there are a few small issues. And such issues can often be combined to produce bigger issues, so it's important to be wary about them.

First, you're not restricting the path of the image to PNG_FOLDER. An attacker can ask for ?id=../../etc/passwd. It will fail because you check that the id ends with '.jpg'. But what if the user sends ?id=../../etc/passwd,some_file.jpg? Did you check that png-to-jpg is not going to try to split on a comma? A simple fix: get a list of all possible file names first and only allow an id which is in the list.

Second, you're sending the full png-to-jpg answer. I don't know how this utility works, but an attacker can probably gain useful information just by looking at the output. He will know that he should look at png-to-jpg vulnerabilites. More generally, any information sent to an attacker is another thing that can be exploited.

Third (this is not necessarily an issue), but it's possible to list all files by simply trying all combinations and see the output. It's even possible to see what's already converted and what's not with timing attacks.

I'm not saying that fixing these issues will make your code fully secure, but it will be a step forward.

StackExchange Code Review Q#140309, answer score: 3