Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix tarfile security issue #434

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

dmarteau
Copy link
Member

Opening untrunsted tarfile is a potential security issue:

Since tarfiles used in cadastre are downloaded from external site, they should be considered as untrusted.

The issue is solved by adding the parameter fllter='data' in the extractall method: see See https://docs.python.org/3.8/library/tarfile.html#tarfile.TarFile.extractall.

IMPORTANT NOTE: the filter parameter require python 3.8 minimum.

Lower version of python are DEPRECATED and should not be supported anymore, since they present a secuity risk.

@rldhont
Copy link
Collaborator

rldhont commented Dec 21, 2023

From docs https://docs.python.org/3.10/library/tarfile.html#tarfile.TarFile.extractall

The filter argument, which was added in Python 3.10.12, specifies how members are modified or rejected before extraction. See Extraction filters for details. It is recommended to set this explicitly depending on which tar features you need to support.

The test is running under python 3.10.6

@dmarteau
Copy link
Member Author

The test is running under python 3.10.6

Then the test image MUST be updated:

The filter parameter fix has been backported in Python version: 3.8.17, 3.9.17, 3.10.12

This is a security fix (see https://www.python.org/downloads/release/python-3817/) and, imho, it is not acceptable to use outdated releases.

@Gustry
Copy link
Member

Gustry commented Dec 22, 2023

Pour les tests, c'est totalement mineur comme souci.

Par contre, c'est pour la prod le problème en l'état actuel du PR, ca va juste mettre une Python stacktrace à l'utilisateur, sans comprendre quoi faire. Soit on trouve les versions min Python dans les différentes versions de QGIS et on monte la version min si on peut se le permettre.

Mais sinon, il faut impérativement soit tester la version de Python, soit un try...except en mettant une note. J'essaie de trouver les versions de Python pour les versions de QGIS (ticket QGIS 54491)
Quelle version min de QGIS impactée ? Suivant l'OS ?

This is a security fix (see https://www.python.org/downloads/release/python-3817/) and, imho, it is not acceptable to use outdated releases.

On est d'accord, mais nous n'avons aucun contrôle sur la version de Python... ce n'est pas de notre resort.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants