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

feat: Add getForbiddenFilenames to \OCP\Util and unify filename sanitizing #45151

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

susnux
Copy link
Contributor

@susnux susnux commented May 2, 2024

Some preparations and refactoring for #44963

Summary

  • Add a common access to the forbidden filename config (now \OCP\Util::getForbiddenFilenames())
  • Cache forbidden filenames and characters, as with 🪟📁📄 Ability to enforce windows compatible file names on the server #44963 the function for calculating those values might be more complex (also the forbidden filenames were already cached on the filesystem class).
  • Drop OC_Util::isValidFilename as it was deprecated for a long time, not used anymore and not checking forbidden filenames
  • Deprecate \OCP\Files\FileInfo::BLACKLIST_FILES_REGEX as this was not used anywhere (only in the util function but that is also not used anywhere + in the files app, but never enforced on webdav).
  • Add the forbidden filenames and forbidden characters to the JS config for frontend validation use

Todo ⚠️ - Needs discussion

  • It was never documented but the deny list of files was somewhat case in-sensitive. Document that or make it case-sensitive? (The first is currently done).
  • The file name blacklist regex was not enforced anywhere, meaning you can currently upload .fileinfo files. So I dropped it, but should be instead enfore it?

Checklist

@susnux susnux added bug 3. to review Waiting for reviews php Pull requests that update Php code labels May 2, 2024
@susnux susnux added this to the Nextcloud 30 milestone May 2, 2024
@susnux susnux added 2. developing Work in progress and removed 3. to review Waiting for reviews labels May 2, 2024
@susnux susnux force-pushed the fix/ensure-blacklist-is-honored branch 7 times, most recently from acff472 to 059efb2 Compare May 2, 2024 20:31
@susnux susnux added 3. to review Waiting for reviews enhancement and removed 2. developing Work in progress bug labels May 2, 2024
@susnux susnux force-pushed the fix/ensure-blacklist-is-honored branch from 059efb2 to 54f89f7 Compare May 2, 2024 20:48
@susnux susnux requested review from icewind1991, blizzz, Fenn-CS, a team and ArtificialOwl and removed request for a team May 2, 2024 21:22
@blizzz blizzz removed their request for review May 3, 2024 08:01
@susnux susnux requested a review from artonge May 3, 2024 14:56
tests/lib/Files/FilesystemTest.php Show resolved Hide resolved
tests/lib/UtilTest.php Show resolved Hide resolved
@susnux susnux requested a review from come-nc June 5, 2024 14:28
@susnux susnux changed the title feat: Add getForbiddenFilenames to \OCP\Util and unify filename sanitizing feat: Unify filename sanitizing with \OCP\Files\FilenameValidator Jun 24, 2024
@susnux susnux changed the title feat: Unify filename sanitizing with \OCP\Files\FilenameValidator feat: Add getForbiddenFilenames to \OCP\Util and unify filename sanitizing Jun 24, 2024
@susnux susnux force-pushed the fix/ensure-blacklist-is-honored branch from 1135b01 to 694995f Compare June 24, 2024 11:05
@susnux susnux force-pushed the fix/ensure-blacklist-is-honored branch 2 times, most recently from 74de2ee to 841d19d Compare July 3, 2024 14:32
@susnux susnux force-pushed the fix/ensure-blacklist-is-honored branch from 841d19d to 994dc7b Compare July 3, 2024 19:34
@susnux susnux force-pushed the fix/ensure-blacklist-is-honored branch from 994dc7b to 1968b74 Compare July 3, 2024 19:42
} elseif (file_exists($src)) {
$validator = \OCP\Server::get(FilenameValidator::class);
if (!$validator->isForbidden(\basename($src))) {
copy($src, $dest);

Check failure

Code scanning / Psalm

TaintedFile Error

Detected tainted file handling
@susnux susnux force-pushed the fix/ensure-blacklist-is-honored branch from 1968b74 to 8d6cf37 Compare July 3, 2024 20:05
@susnux susnux requested a review from skjnldsv as a code owner July 3, 2024 20:05
@susnux susnux marked this pull request as draft July 3, 2024 20:07
@susnux susnux force-pushed the fix/ensure-blacklist-is-honored branch 3 times, most recently from e3dc322 to 5e160c0 Compare July 3, 2024 21:41
@susnux susnux force-pushed the fix/ensure-blacklist-is-honored branch from 5e160c0 to a5817ca Compare July 5, 2024 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement feedback-requested php Pull requests that update Php code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🪟📁📄 Ability to enforce windows compatible file names on the server
3 participants