Fix improper method call mktemp

[fazledyn-or]

Details

While triaging your project, our bug fixing tool generated the following message(s)-

In file: generators.py, there is a method that creates a temporary file using an unsafe API mktemp. The use of this method is discouraged in the Python documentation. iCR suggested that a temporary file should be created using mkstemp which is a safe API. iCR replaced the usage of mktemp with mkstemp.

Resources Related to mktemp

• POC - Improper Method Call - python - mktemp.mp4
• MISC - Python - Taking a peek under the tempfile library.mp4

Changes

• Replaced mktemp() method with mkstemp()
• Closed the opened file-descriptor in file spikeinterface/src/spikeinterface/extractors/mdaextractors.py

Previously Found & Fixed

• https://www.github.com/spcl/dace/pull/1428
• https://www.github.com/invesalius/invesalius3/pull/679
• https://www.github.com/Azure/azure-linux-extensions/pull/1816
• https://www.github.com/celery/billiard/pull/394

CLA Requirements

This section is only relevant if your project requires contributors to sign a Contributor License Agreement (CLA) for external contributions.

All contributed commits are already automatically signed off.

The meaning of a signoff depends on the project, but it typically certifies that committer has the rights to submit this work under the same license and agrees to a Developer Certificate of Origin (see https://developercertificate.org/ for more information).
- Git Commit SignOff documentation

Sponsorship and Support

This work is done by the security researchers from OpenRefactory and is supported by the Open Source Security Foundation (OpenSSF): Project Alpha-Omega. Alpha-Omega is a project partnering with open source software project maintainers to systematically find new, as-yet-undiscovered vulnerabilities in open source code - and get them fixed – to improve global software supply chain security.

The bug is found by running the Intelligent Code Repair (iCR) tool by OpenRefactory and then manually triaging the results.

[h-mayorquin]

Hi, Thanks a lot your contribution.

changing the mktemp method makes sense. Can you elaborate on the problem with the file descriptor?

[fazledyn-or]

Hi, Thanks a lot your contribution.

changing the mktemp method makes sense. Can you elaborate on the problem with the file descriptor?

Certainly!

In Linux, each processes are allocated 1024 file descriptors by default. Among those, 0 to 2 are assigned to stdin, stdout, and stderr by default. As a result, only 1021 more file descriptors can be opened by the process itself. Opening files using file-descriptors but not closing them can lead to a file-descriptor exhaustion. CWE-7775

For example -

from tempfile import mkstemp

def call_mkstemp():
    fd, name = mkstemp()

for i in range(1100):
    print(i, end=", ")
    call_mkstemp()

Executing this code will result in an error when the 1022nd file desciptor is asked to be opened. Since the fd isn't getting closed, it's resulting in a file descriptor leak. As a good practice, we must close the file-descriptors that we open ourselves.

[alejoe91]

Thanks @fazledyn-or

[fazledyn-or]

Is it okay to be merged?

[h-mayorquin]

It is fine by me, @samuelgarcia , anything you remember about why that descriptor is open? I don't know whether you or @alejoe91 ported that code.

[samuelgarcia]

Hi.
Thanks for this.
Coudl we have a better approach wit a with: rather than an explicit os.close ?

[fazledyn-or]

I'm afraid that there's no context manager available for mkstemp method. See, the mkstemp method itself opens the fd and returns the value to us. It's upon us to close it. We have even seen other open source projects doing it (os.close(fd))

1. However, there's another approach to this by using NamedTemporaryFile. In that case, you can use a context manager.

with tempfile.NamedTemporaryFile() as temp:
    temp.write('Some data')
    temp.flush()

2. If you're still interested in using mkstemp, then doing something as below won't require an os.close(fd) statement

from tempfile import mkstemp
fd, name = mkstemp()
with open(fd, "w") as f:
    f.write("something")

3. Same approach as 2 but using os.fdopen() to open the file.

References

• https://github.com/python/cpython/blob/main/Lib/tempfile.py#L521
• https://docs.python.org/3/library/os.html#os.fdopen

[alejoe91]

Let's merge this but change to context with tempfile.TemporaryFile() later @h-mayorquin

[h-mayorquin]

Thanks for your contribution @fazledyn-or!