ANN: Snapcraft 4.4.4 (library injection vulnerability on built snaps)

snapcraft
#1

Hello snapcrafters! The Snapcraft team announces version 4.4.4.

New in this release:

Core

Exclude current directory from LD_LIBRARY_PATH

Earlier releases of Snapcraft included the current directory when configuring LD_LIBRARY_PATH for application commands. This could have unintended consequences for strict mode snaps under certain circumstances. (CVE-2020-27348, https://usn.ubuntu.com/4661-1/)

This new Snapcraft releases ensure that LD_LIBRARY_PATH no longer contains the current directory and simply rebuilding the snap with the newer Snapcraft will resolve the issue. While this change should be safe for the majority of snaps, snaps that relied on the previous behavior may behave differently (eg, if the snap changes into a directory with libraries that weren’t already covered by other parts of LD_LIBRARY_PATH). If your snap relies on the previous behavior, simply adjust LD_LIBRARY_PATH to include all the required directories your application needs.

2 Likes
#2

Related announcement https://discourse.ubuntu.com/t/usn-4661-1-snapcraft-vulnerability/19640

#3

@sergiusens should the docker images also be respun?

1 Like
#4

Sorry, my reply yesterday did not make it through as a thunderstorm was developing and Internet broke down. I did regenerate those yesterday too.

1 Like
#5

Rebuilding some snaps today I notice that one generated an error relating to this CVE even after the rebuild. It turns out that in the snapcraft.yaml I’d defined:

environment: 
 LD_LIBRARY_PATH: ${SNAP}/usr/local/lib:${LD_LIBRARY_PATH}

In my mind this made sense, since I was assuming that the original variable was defined first and therefore I was simply bumping the front portion infront of it. It actually turns out this isn’t the case at all and should just have been

LD_LIBRARY_PATH: ${SNAP}/usr/local/lib.

The new code checking for :: managed to also identify this scenario, but I reckon that there might be people who might have made the same assumption I did.

Snapcraft did give the correct warning

CVE-2020-27348: A potentially empty LD_LIBRARY_PATH has been set for environment in '.'. The current working directory will be added to the library path if empty. This can cause unexpected libraries to be loaded.

And of course the warning when running the snap itself also appeared,

I’m wondering if it might be worth checking for this exact condition I had and have specific feedback on it. Whilst the CVE is fixed from the snapcraft tooling side itself, I think there might still be people like me who manage to screw it up, because it’s not clear at first glance the order the variables are parsed in when defined in the yaml itself.

3 Likes
#6

Excellent, thank you, looks good now.

#7

Just as a followup to my prior post, I took a further glance into what was going on and looked to see if anyone else made my same mistake. Going through the Snapcrafters repo, GIMP and ScummVM both have this issue, https://github.com/snapcrafters/gimp/blob/50ef901c4c4e945cee3f3022c93ed5679c836d31/snap/snapcraft.yaml#L108 and https://github.com/snapcrafters/scummvm/blob/44b581b0f117a75c65f95580192c887554d7a60d/snap/snapcraft.yaml#L48

Of course, there’s probably a lot more instances of it throughout the entire snap sure, I just wanted to be sure it wasn’t just me with this conception. I built the snaps on Snapcraft 4.4.4 to confirm the behaviour is as described even on the new release.

This seems like something best handled at the review-tools or snapcraft level rather than sending patches individually to projects.

1 Like
#8

We cannot auto patch projects, this might lead to undesired behavior even if in most cases it is not the intended case.

#9

I can understand not automatically patching projects since the maintainer would have no awareness, I was thinking more just extend the current snapcraft error and introduce a hard failure for my specific issue. The fix is trivial to the maintainer but without it then for a certain amount of snaps this CVE may continue to exist indefinitely. If the maintainer intended CWD then they should be required to change to :: explicitly and not $LD_LIBRARY_PATH:$ETC (or as you say above, specificy the paths explicitly rather than rely on CWD at all)

#10

Thanks a lot @James-Carroll for pointing this out and already providing a fix to the ScummVM snap.

I agree that there should be some way to raise awareness of this issue besides having to manually review each and every snap. Would it be possible to use a similar mechanism that already exists for reporting outdated packages by mail?

#11

Can we introduce a warning or failure case for instances where LD_LIBRARY_PATH is being overridden with a reference to the unset self. i.e. if the environment definition looks like below with a reference to $LD_LIBRARY_PATH or ${LD_LIBRARY_PATH} anywhere in the string:

environment:
  LD_LIBRARY_PATH: ...:$LD_LIBRARY_PATH:...
#12

There should be a warning: https://github.com/snapcore/snapcraft/pull/3345/files#diff-666cccfeb78a4451ed96b7f6937a030e778affb958c226634f617c2518ed65ebR495

If you’re not seeing one where you would expect, please let us know :smiley:

1 Like
#13

Is it known if that warning appears on classic confinement snaps?

I tried build https://github.com/snapcrafters/jenkins the other day, it didn’t present the warning in snapcraft despite having $LD_LIBRARY_PATH in its LD_LIBRARY_PATH definition. I presume it’s running with a trailing colon and thus is including CWD too, but I didn’t check its actual runtime environment to see if it definitely had a trailing colon. (Importantly though, unlike strict snaps which will complain about including CWD when built in 4.4.4; classic snaps don’t have the same check in their snapcraft_runner).

Edit: Looking at the commit it doesn’t appear classic mode was intended to be checked by this, is this not considered a flaw in the classic context?

#14

No it is is not, and classic confinement will pass through the LD_LIBRARY_PATH set in the environment:

sergiusens@gotham:~$ LD_LIBRARY_PATH=foo snap run --shell snapcraft
sergiusens@gotham:~$ echo $LD_LIBRARY_PATH
foo
1 Like
#15

I’ve set the following environment and encountered the CVE-2020-27348: A potentially empty LD_LIBRARY_PATH has been set for environment in 'gallery-dl'. The current working directory will be added to the library path if empty. This can cause unexpected libraries to be loaded. warning, is there anyway to fix this?

    environment:
      # Satisfy FFmpeg's libpulsecommon dependency
      LD_LIBRARY_PATH: $LD_LIBRARY_PATH:$SNAP/usr/lib/$SNAPCRAFT_ARCH_TRIPLET/pulseaudio
#16

Remove the initial $LD_LIBRARY_PATH, for confined snaps (!classic) it should always be empty. If not, use variable expansion. It is also a warning, if for some corner case reason you really need it, just add a comment on why in your snapcraft.yaml with the reasoning so it is not forgotten.

1 Like
#17

@sergiusens, I haven’t tested this, but does the following work?

LD_LIBRARY_PATH: ${LD_LIBRARY_PATH:+$LD_LIBRARY_PATH:}$SNAP/usr/lib....

and for the variable expansion on the other end:

LD_LIBRARY_PATH: $SNAP/usr/lib....${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}

(I might have the ${VARIABLE:+} syntax wrong)

If that syntax works, could we fix the CVE silently by massaging when we detect a problem? (I suspect we can only reliably do so when the LD_LIBRARY_PATH is the very first or very last entry)