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

Prevent importing files outside import paths #1897

Closed
gimenete opened this issue Feb 24, 2014 · 5 comments
Closed

Prevent importing files outside import paths #1897

gimenete opened this issue Feb 24, 2014 · 5 comments

Comments

@gimenete
Copy link

We are thinking to include Less in a software-as-a-service where users can edit their own templates.

We've made a simple test to prevent users @import sensitive files.

var parser = new(less.Parser)({
    paths: ['./less_files'],
    filename: 'style.less'
})

parser.parse('@import "../forbidden.less";', function (e, tree) {
    var output = tree.toCSS({ compress: true }) // Minify CSS output
    console.log('output', output)
})

This works so they would be able to import files outside their directories. Is there any way to prevent this behavior? It would be great to have something like a 'safeImports" flag that would prevent this.

We know that the parser fails if the imported file is not a valid less file, but we would like to prevent the parser even to read the file. Is that possible?

Thank you.

@lukeapage
Copy link
Member

Cool. You may want to switch off JavaScript support too.

w.r.t. imports if I were you I would look at restricting access the app has
so it can't access files outside a particular folder - even if I have add
an option, it might be we introduce a bug or there is something the user
can do to get round it - and its open source code, so a malicious user is
free to look for vulnarabilities.

Having said that I'm working on plugin infrastructure and we would like to
support requests for non standard options and behaviour through plugins. In
our current design it will be quite easy to write a plugin to check all
imports. At the moment its difficult only because we cannot inject visitors
before the import phase.

@gimenete
Copy link
Author

Thanks @lukeapage

We have been discussing internally this issue and we have in mind an approach much stronger in security terms but more complicated. Executing the parser inside a sandbox using the vm module of Nodejs and faking the fs module to proxy the filesystem calls. So we would have the ability to prevent any malicious access despite of any possible bug in less. Maybe we end up with that solution even though it is more complicated and CPU and memory expensive.

Nevertheless that plugin infrastructure would be great.

Thank you!

@gimenete
Copy link
Author

gimenete commented Mar 5, 2014

Hi,

Just if you are curious I have made a proof of concept sandboxing less and making a fake fs module. Here is the gist https://gist.github.com/gimenete/9377962 In that code the files need to be in basedir, and the code prevents to import files outside that directory or its descendants. The parser has only access to the url and path modules and it is very limited in the use of the fs module.

In my machine compiling bootstrap 100 times takes 9.3 seconds, while using less directly it takes 6.7 seconds. So not a big penalty I think.

@hugdx
Copy link

hugdx commented Feb 3, 2015

Thank @gimenete, this is path for open_basedir https://gist.github.com/hugdx/33b0060431f8f20ad1a7

@stale
Copy link

stale bot commented Nov 14, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

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

No branches or pull requests

3 participants