@import allows dumping of file content of almost anyfile.

[ankitagarwal]

@import should verify a file's content before importing it.

Symlink files are dumped straight in css irrespective of their content. In our tests we used -
'@import "../../../../../../../home/jake/testing/test.php"'
Which refers to '/home/jake/moodledata/master/moodle/theme/boost/scss/../../../../../../../home/jake/testing/test.php'

This happens only when webroot has a different path to actual directory. For example -
/var/www/html/master = 4 vs /home/jake/moodledata/master/moodle = 5

Besides this ideally any file with an invalid extension should not be allowed (such as .php .config, etc)

[robocoder]

Not exactly. More than likely, scssphp will throw an exception as it can't parse abitrary non-scss/css files.

From a security perspective, scssphp can mitigate, but as a library, it's really the responsibility of the consumer if it allows the arbitrary upload and compilation of untrusted content.

[snake]

The problem reported here is related (and can only be exploited in) a webroot that has been symlinked to another directory somewhere (with different depth relative to /). It is not an issue with file extensions. I originally reported it to Ankit. I believe the above patch to Compiler.php doesn't solve this because the problem only occurs when the findImport() call returns false; adding an additional check for file extension won't help as it will still return false. I'll do my best to explain below.

The problem lies is the checking of files to compile. When doing this, compileChild() checks compileImport() to see whether or not the file can be compiled in. If it can't, an '@import' statement is added. Tracing the calls through, compileImport() uses findImport() to check whether the file exists. If findImport() can't find the file, it returns false. This is the crucial part. If the file can't be found, findImport() returns false resulting in a blind '@import' statement being added by compileChild().

In our case, findImport() looks for the file with respect to the linked file system (i.e. NOT /var/www/html/) and can't find the file. Relative to that location, the file doesn't exist. The @import statement is added instead. Then, when viewing the scss file in the browser, the content of whatever was in the file is added to the scss output. I suspect this is because the '@import' statement looks for the file with respect to '/var/www/html/' , and it can now be located and added.

So, whilst this is a rather obscure case, checking the extension in Compiler->findImport() won't solve it for us. The reason we noticed it is that we allow scss uploads for theming purposes, and need to be 100% sure that scss parsing can't be exploited like in the above example.

[robocoder]

Two suggestions:

1. Wrap your compile() call inside a try...catch, so you can avoid disclosing any sensitive data that might be exposed if an exception is thrown.

2. Before calling the compile() method, chdir to a build folder and restrict the open_basedir configuration at runtime to prevent @import outside of the build folder.

[robocoder]

Ah, I see what you've done in https://github.com/moodle/moodle/blob/master/lib/classes/scss.php. I don't plan to add a setting or callback to validate/filter import paths.

I think your implementation would be cleaner if overrode the findImport method instead.