Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Fix #13099: Disallow user to create files/folder using relative path #13256

Merged
merged 7 commits into from Feb 20, 2018
Merged

Fix #13099: Disallow user to create files/folder using relative path #13256

merged 7 commits into from Feb 20, 2018

Conversation

sdalmeida
Copy link
Contributor

@sdalmeida sdalmeida commented Apr 2, 2017

This is a Work In Progress PR referring to original issue #13099
Before this PR can be landed, it would be nice to know which chars are actually invalid.

These are the chars that I'm invalidating:

  • \?
  • \\
  • \*
  • \.{2,} <-- .. or more should not be allowed, but allow .
  • \.$ <-- allow a file to have . but not end with .
  • \/
  • \|
  • \>
  • \<

I also have to add a unit test for this case.
Let me know if there are any chars that need to be added/removed.

@swmitra
Copy link
Collaborator

swmitra commented Apr 2, 2017

@Simon66 I still have the same confusion regarding invalid character handling code. Part of the reason is the way file names get validated by the OS, it's quite different in case of windows and osx. Not sure whether we can have a common regex validator as that would limit the file naming capability in either of the platform.

@swmitra
Copy link
Collaborator

swmitra commented Apr 2, 2017

What we can think about is, predominantly the projects being worked upon are web projects in Brackets. Eventually these files will be served and loaded in browser shells. What kind of restrictions do we have in the file names while serving them over network. Just thinking loud, may be not valid in this context.

@sdalmeida
Copy link
Contributor Author

Another confusion of mine is why are these filenames considered "bad"

screen shot 2017-04-02 at 1 23 23 pm

Why is com a valid filename and com1 not?
Also, I don't see why any of these file names are labelled as invalid.
Any ideas @tallandroid?

@sdalmeida
Copy link
Contributor Author

@swmitra I think that no matter where this code lives (MacOS, Windows, Linux), those chars should be invalid. I'm not too sure why we should allow < or > on a linux env but not on Windows (Even though that char should be invalid on both OS.
But then again, which chars are invalid and which should not be.

@sdalmeida
Copy link
Contributor Author

I've changed the function isValidFilename to disallow any file name that contains folder structure (like tmp\file.log) and I created a new function to check if full pathname is valid. I've also added some test cases.

@sdalmeida sdalmeida changed the title [WIP] Fix Issue 13099 Fix #13099: Disallow user to create files/folder using relative path Apr 7, 2017
new RegExp("[" + invalidChars + "]+").test(filename) ||
_illegalFilenamesRegEx.test(filename)
filename.match(_invalidChars)|| filename.match(_illegalFilenamesRegEx)
//filename.match(_invalidChars) || filename.match(_illegalFilenamesRegEx)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This got left in by accident, please remove.

@sdalmeida
Copy link
Contributor Author

@humphd Done 👍

@ficristo ficristo requested review from saurabh95 and swmitra May 2, 2017 18:15
@sdalmeida
Copy link
Contributor Author

sdalmeida commented Feb 8, 2018

Hi @swmitra
I want to follow up on the status of this PR. I see that the code was approved but the PR is still open. There are many forked projects that might benefit from this bug fix 👍
Thanks :)

@nethip nethip merged commit 51a5908 into adobe:master Feb 20, 2018
@nethip
Copy link
Contributor

nethip commented Feb 20, 2018

Thanks for your contributions @Simon66 .

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

Successfully merging this pull request may close these issues.

5 participants