search

3liz / QgisCadastrePlugin

A QGIS plugin which helps users to import the french land registry ('cadastre') data into a database. It is meant to ease the use of the data in QGIS by providing search tools and appropriate layer symbology.
GNU General Public License v2.0
61 stars 41 forks source link

Fix tarfile security issue #434

Closed dmarteau closed 3 months ago

dmarteau commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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.

Gustry commented 3 months ago

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

Still 40% of the plugin uses Python 3.9.5 :(

The PR #457 doesn't make any exception if the Python version is too low.