nodejs/node

fs.lchmod opens file with O_WRONLY unnecessarily, fails on directories and non-writable files

Open

#23 736 ouverte le 18 oct. 2018

Voir sur GitHub
 (3 commentaires) (0 réactions) (0 assignés)JavaScript (35 535 forks)batch import
fshelp wantedstale

Métriques du dépôt

Stars
 (117 218 stars)
Métriques de merge PR
 (Merge moyen 18j 17h) (219 PRs mergées en 30 j)

Description

  • Version: v10.12.0
  • Platform: Darwin geegaw.local 18.0.0 Darwin Kernel Version 18.0.0: Wed Aug 22 20:13:40 PDT 2018; root:xnu-4903.201.2~1/RELEASE_X86_64 x86_64
  • Subsystem: fs

https://github.com/isaacs/chmodr/pull/20

Node's fs.lchmod implementation opens the file in write-only mode. (On Darwin, at least.)

This is unnecessary, and fails for read-only files.

Additionally, this approach (open and then use fchmod) fails for directories. Is there a reason why native lchmod isn't being used? Systems that have O_SYMLINK also have lchmod(3), don't they?

This is important because lchmod is the only way to avoid a (minor) security vulnerability when doing recursive mode setting on directories. If we have to restrict the use of lchmod to only symlinks, then we're back in TOCTOU territory.

Guide contributeur

Direction de recherche
Examinez l'implémentation de fs.lchmod dans le code source de Node.js (src/node file.cc). Déterminez si la plateforme fournit un lchmod natif et, le cas échéant, utilisez le directement au lieu de open+O WRONLY. Considérez également le cas des répertoires.
Stack technique
nodejs
Domaine
backend
Type d'issue
Bug
DifficultéDifficulté estimée pour un nouveau contributeur, de 1 pour un très petit changement à 5 pour un travail expert.
3
Temps estiméFourchette de temps approximative pour investiguer, implémenter, tester et préparer une pull request.
Une demi journée
Statut d'activitéDisponibilité apparente de l'issue : fraîche, active, ancienne, bloquée ou en attente d'un mainteneur.
Ancienne
ClartéClarté avec laquelle l'issue explique le changement attendu, les critères d'acceptation et la prochaine étape.
Claire
Prérequis
C
Accessibilité débutantScore de 1 à 100 estimant l'accessibilité de cette issue pour un premier contributeur.
30