Loading arduino-cli.yaml from current directory might be unexpected or unsafe

[matthijskooijman]

First off: even though I consider this a security problem, I do not think this is very easy to exploit (unnoticed). I've reported this issue tot the Arduino security staff privately first, and we agreed that this would be acceptable for public discussion. Hence this issue.

I noticed (after reading #699 (comment)) that arduino-cli allows loading its arduino-cli.yaml config file from the current directory, or any of the parent directories (e.g. see here and here).

However, I'm a bit worried about the (security) implications of this. It seems that this would make it very easy to mess up arduino-cli when a arduino-cli.yaml file is present in an unexpected location.

From a security standpoint, this means that arduino-cli can never be ran from a untrusted directory, which I think users will not expect.

For example, consider a sketch downloaded from elsewhere. Normally, compiling an untrusted sketch is not problematic, since the files contained in the sketch are only passed to the compiler, not actually executed on the compiling machine. Uploading a sketch with untrusted code to an Arduino can of course be problematic, but Arduinos are usually sufficient sandboxed, and a user can expect that untrusted code will be ran in this case.

Now, consider that this untrusted sketch also contains an arduino-cli.yaml file, that sets up the user directory to a directory inside the sketch. If it then also contains a hardware directory with a custom core, then running arduino-cli compile will execute arbitrary commands from the recipes in the untrusted platform.txt file. These could then do anything, including removing files, installing a backdoor or stealing data.

As for fixing this, here's some thoughts:

1. You can think of an approach where not all configuration directives will be accepted in these potentially untrusted "project" config files (i.e. divide all config directives into two groups, one needing a trusted config file and the other allowed from any config file). However, this is often fragile, when new options are added or refactored, it is easy to forgot to realize that an option can cause a security problem.

2. Hence, it might be better to consider an approach where a project config file must be explicitly allowed (e.g. when an arduino-cli.yaml exists in the current directory, arduino-cli should ask the user whether it is trusted, maybe with a short explanation of the risks involved). Then, for each file (maybe along with a checksum), the answer (allow or ignore) should be stored. This is an approach also used by tools like direnv or githooks.

3. Or maybe disable these project files by default, and enable them in the global arduino-cli.yaml config file.

Note that all of the above suggestions require that the e.g. ~/.arduino15/arduino-cli.yaml global file should always be loaded (either to get values for the trusted-only directives, to store the trusted project config files, or to store the "allow project config" setting). That also opens up the door to loading the global file for defaults, and letting a project config file only override some values, as needed, which reduces the chances of a project config file from accidentally changing settings it does not really need to change.

The essence of this issue has been considered before, which is (I think) one of the reason why per-sketch compilation options were never implemented in the Java IDE (e.g. see arduino/Arduino#421 (comment)).

[ubidefeo]

Thank you @matthijskooijman
This is a lot of good material I'll reference to in the next internal review.
We are in the process of clarifying and distinguishing between the CLI configuration settings and the sketch ones and your remarks touch on some of my points.

I hope we can turn this around quickly :)
u.

[matthijskooijman]

@ubidefeo Any progress on this issue? You mentioned working on configuration options two years ago, but did that ever reach a conclusion? I think it would be good to fix this issue, being a potential security problem.

Looking back at my suggested possible fixes, I would hope something like option 2 can be implemented.

I'm actually using this feature now to allow bundling a boards package/core with a sketch (through a submodule) by pretty much exactly the exploit I sketched above: Add a arduino-cli.yaml file inside the sketch that sets the sketchbook directory to be the sketch directory so the hardware and libraries subdirectories are used. With option 2, this can remain working like that in a safe way. An alternative would be to allow using libraries and hardware folders from inside the current sketch directories by default (without needing a custom arduino-cli.yaml file), but even then hardware folders should only be used when explicitly enabled to prevent this same security problem.

[ubidefeo]

@matthijskooijman

We decided to disallow the automatic use of any locally available arduino-cli.yaml.
If a user wants to use a specific, local arduino-cli.yaml on a project basis, this must be supplied in the command line.

i.e.:
arduino-cli --config-file ./arduino-cli.yaml compile -b arduino:samd:nano_33_iot -v

[matthijskooijman]

Sounds like a good approach. And if a user wants to automatically apply this to all directories (i.e. restore the current behaviour except for parent directories), they can just use alias arduino-cli=arduino-cli --config-file ./arduino-cli.yaml in their bashrc or so.

[ubidefeo]

@matthijskooijman

DEAL!