snapd fails to validate content interface settings, resulting in sandbox escape

[Ian Johnson]

Snapd does not properly or sufficiently validate the input strings used in content interface plugs/slots, meaning that snaps can be installed with strict confinement with malformed content interface slots which effectively grant any AppArmor rule to the plug side.

To exploit this, create two snaps, one which provides the slot and one which has a plug connected to the slot. For the purposes of the exploit, these can just be local snaps, but note that a snap publisher could upload both of these snaps to the store and the two snaps will have their plug/slot auto-connected due to rules about auto-connection of matching content interface plugs/slots for snaps of the same publisher.

The first snap has a content slot definition like this:

slots:
  content-plug:
    interface: content
    content: mycont
    read:
      - "$SNAP/ rw, /** rw, } profile foobar (attach_disconnected) { /foo"

The embedded profile name does not really matter, but what is important is that this rule has arbitrary apparmor rules embedded inside it, abusing the "," character. For the plug side use a definition like this:

plugs:
 content-plug:
  interface: content
  content: mycont
  target: $SNAP_DATA/mycont

The plug side target does not matter at all, but the `content` attribute must match the slot definition in order for the plug and slot to be auto-connected.

What we are effectively doing is taking advantage of the fact that snapd just validates that the read setting is a "clean" filepath, and does no further validation and effectively ends up just copy-pasting the string into various places inside an AppArmor profile without any quoting or further validation.

There are really two profiles which get generated with this malicious string without validation, the one for snap-update-ns of the plugging snap in question, and the one for the snap itself. This actually presented something of a problem, as for snap-update-ns, the string appears as part of a mount rule source, which means that including other stuff here like we do makes apparmor_parser fail to compile the file for snap-update-ns, as it does not expect a mount rule to be formed like this. I still suspect it is possible to craft a string such that it somehow is valid both as a file source in a mount rule and as a file rule itself, but it turned out that actually it doesn't need to be a valid rule for both profiles in order to be exploited. This is because snapd just loads the profiles and if they fail to be loaded, snapd does nothing about it. This means we can craft a rule which is valid for just the profile for the app itself, (but not for the profile of snap-update-ns), and still be able to use our crafted rule. The one hiccup to this is that the mount namespace must already exist, so that snap run does not need to invoke snap-update-ns, otherwise presumably the exploit will not be exploitable since the app cannot be run. It might be possible to also avoid this by using a daemon and something like refresh-mode: endure, but I didn't take the time to figure out all those details.

To be clear, with the above plug, for the plugging app snap we get this as the tail end of the apparmor profile:

```
# In addition to the bind mount, add any AppArmor rules so that
# snaps may directly access the slot implementation's files
# read-only.
/snap/test-content-interface-escape-slot/x15/ rw, /** rw, } profile snap-update-ns.test-content-interface-escape-plug2 (attach_disconnected) { /foo/** mrkix,

}
```

Which for the purposes of this bug just demonstrates that we can inject arbitrary apparmor rules into the profile through the snap.yaml plug/slot definition.

Unfortunately, fixing it isn't quite as simple as just quoting the input string, as trying to compile this:

"/snap/test-content-interface-escape-slot/x15/ rw, /** rw, } profile snap-update-ns.test-content-interface-escape-plug2 (attach_disconnected) { /foo/**" mrkix,

actually fails as a rule. It's unclear to me how much AppArmor profiles really support quoting things, so the proper fix for this may be to start enforcing stricter patterns of supported filepaths, and excluding paths with commas or braces in them for example.

We may also want to consider rolling back AppArmor profiles if they fail to be compiled to prevent issues like one profile of a set being compiled successfully, but another failing to. Currently we leave these profiles around on disk in whatever state we tried to write them as until they are overwritten by a new update, etc.

[Ian Johnson (anonymouse67)]

Note that running review-tools on the snap with malicious slot produces "OK", so uploading such a snap to the store today would likely be allowed.

[Ian Johnson (anonymouse67)]

We are going through the other interfaces in snapd which take developer strings from the snap.yaml for interface definitions, and have validated that while these other interfaces take input from the snap.yaml, they are properly validated against very specific regular expressions and thus not subject to the same injection:

* serial-port
* iio
* i2c
* bool-file
* hidraw
* uio

However, the system-files, and personal-files interfaces both allow arbitrary paths as input and thus are also subject to this same vulnerability as the content interface, but these interfaces are super-privileged and closely monitored in the store, so they are unlikely to be able to be abused. We should still probably adjust code inside snapd to be more strict in what snapd allows, and also to quote the filepath rather than include it directly.

Finally, the netlink-driver interface has a weird quirk upon closer inspection that should be closed independently: the "family-name" attribute has this as the regular expression:

^[a-z]+[a-z0-9-]*[^\-]$

which allows any single character (except "-") at the end, meaning that for example a "," could be injected, though this would likely just make the profile fail to compile and doesn't seem to have a possibility to be used for sandbox escape, since only a single character can be injected into the file.

[Alberto Mardegan (mardy)]

A solution is being worked on at https://github.com/mardy/snapd-private/pull/1

I added a few people to the private repository, but please let me know if you don't have access.

[Seth Arnold (seth-arnold)]

Nice find :) Please use CVE-2021-4120 for this issue.

Thanks

[Seth Arnold (seth-arnold)]

Hey Mardy, could you please add me? https://github.com/setharnold

Thanks

[Alex Murray (alexmurray)]

FYI - this also applies to layouts too - eg:

layout:
  "/var/lib/foo rw, /** rw,#":
    bind: "$SNAP/foo"

like in the original case for content interfaces, again the snap-update-ns apparmor profile ends up malformed but the actual snap's profile is valid.

[Alberto Mardegan (mardy)]

Seth: I've added you now.

Alex: In the last few commits I addressed layouts, too. But please double check :-)

[Alex Murray (alexmurray)]

This was resolved via the following snapd updates in Ubuntu:

https://launchpad.net/ubuntu/+source/snapd/2.54.3+21.10.1

https://launchpad.net/ubuntu/+source/snapd/2.54.3+20.04

https://launchpad.net/ubuntu/+source/snapd/2.54.3+18.04

And by the 2.54.3 release of snapd on github: https://github.com/snapcore/snapd/releases/tag/2.54.3