swagger-api / swagger-api/swagger-codegen

python codegen __deseralize_file() is not safe

Open
#11,704 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Mustache
Stars
17.8k
Forks
6k
PR merge metrics
No merged PRs in 30d

Description

Description

In api_client.py generated by swagger-codegen the function __deserialize_file calls tempfile.mkstemp(dir=self.configuration.temp_folder_path). This returns a unique temporary path. However, the uniqueness is only guaranteed if the full path including the filename is sued. I tested this on macOS 12.1/python 3.8.2 (not sure other os/python combos). A few lines later the code throws away the file name with a call to os.path.dirname(path) and append the filename retrieved from the Content-Disposition header.

This is flawed in two ways:

  1. As I mentioned earlier, if multiple files are downloaded where the filename is the same in the Content-Disposition header then the temp file created will be overwritten, and
  2. It is not a good security practice to trust the filename provided by the server.

The fix is to simply use the full path returned by mkstemp.

Swagger-codegen version

the latest version downloaded now at the time of writing which is 3.0.31

see https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/main/resources/python/api_client.mustache#L524

Swagger declaration file content or url

N/A

Command line used for generation

N/A

Steps to reproduce

N/A

Related issues/PRs

N/A

Suggest a fix/enhancement

suggested change to modules/swagger-codegen/src/main/resources/python/api_client.mustache

    def __deserialize_file(self, response):
        fd, path = tempfile.mkstemp(dir=self.configuration.temp_folder_path)
        os.close(fd)

        with open(path, {{^writeBinary}}"w"{{/writeBinary}}{{#writeBinary}}"wb"{{/writeBinary}}) as f:
            f.write(response.data)
        return path

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in modules/swagger-codegen/src/main/resources/python/api_client.mustache at the __deserialize_file template referenced by the issue. Read how tempfile.mkstemp, the Content-Disposition filename, and the returned path are used. Done means generated Python clients retain the unique temporary path without trusting or replacing its filename.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
tooling
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.