diff -Nru snapcraft-2.43.1/debian/changelog snapcraft-2.43.1+16.04.1/debian/changelog --- snapcraft-2.43.1/debian/changelog 2018-08-30 00:08:05.000000000 +0000 +++ snapcraft-2.43.1+16.04.1/debian/changelog 2020-12-01 12:15:12.000000000 +0000 @@ -1,3 +1,15 @@ +snapcraft (2.43.1+16.04.1) xenial-security; urgency=medium + + [ Sergio Schvezov ] + * SECURITY UPDATE: library injection vulnerability on strict mode + snaps built with snapcraft via misconfigured LD_LIBRARY_PATH + - project_loader: do not export empty environment + - meta: do not export empty environment. Warn on empty environment. + - CVE-2020-27348 + - LP: #1901572 + + -- Emilia Torino Tue, 01 Dec 2020 09:14:20 -0300 + snapcraft (2.43.1) xenial; urgency=medium [ Sergio Schvezov ] diff -Nru snapcraft-2.43.1/snapcraft/formatting_utils.py snapcraft-2.43.1+16.04.1/snapcraft/formatting_utils.py --- snapcraft-2.43.1/snapcraft/formatting_utils.py 2018-08-21 15:49:43.000000000 +0000 +++ snapcraft-2.43.1+16.04.1/snapcraft/formatting_utils.py 2020-12-01 12:13:23.000000000 +0000 @@ -39,15 +39,21 @@ :param str prepend: String to prepend to each path in the definition. :param str separator: String to place between each path in the definition. """ - if not paths: raise ValueError("Failed to format '${}': no paths supplied".format(envvar)) - return '{envvar}="${envvar}{separator}{paths}"'.format( - envvar=envvar, - separator=separator, - paths=combine_paths(paths, prepend, separator), - ) + combined_paths = combine_paths(paths, prepend, separator) + + if separator.isspace(): + formatted = '{envvar}="${envvar}{separator}{combined_paths}"'.format( + envvar=envvar, separator=separator, combined_paths=combined_paths + ) + else: + formatted = '{envvar}="${{{envvar}:+${envvar}{separator}}}{combined_paths}"'.format( + envvar=envvar, separator=separator, combined_paths=combined_paths + ) + + return formatted def humanize_list( diff -Nru snapcraft-2.43.1/snapcraft/internal/meta/_snap_packaging.py snapcraft-2.43.1+16.04.1/snapcraft/internal/meta/_snap_packaging.py --- snapcraft-2.43.1/snapcraft/internal/meta/_snap_packaging.py 2018-08-21 15:49:43.000000000 +0000 +++ snapcraft-2.43.1+16.04.1/snapcraft/internal/meta/_snap_packaging.py 2020-12-01 12:13:23.000000000 +0000 @@ -110,6 +110,7 @@ packaging.setup_assets() packaging.generate_hook_wrappers() packaging.write_snap_directory() + packaging.warn_ld_library_paths() return packaging.meta_dir @@ -404,6 +405,54 @@ self._record_manifest_and_source_snapcraft_yaml() + def warn_ld_library_paths(self) -> None: + root_environment = self._config_data.get("environment", dict()) + root_ld_library_path = root_environment.get("LD_LIBRARY_PATH") + # Dictionary of app names with LD_LIBRARY_PATH in their environment. + app_environment = dict() + + for app_name, app_props in self._config_data.get("apps", dict()).items(): + with contextlib.suppress(KeyError): + app_environment[app_name] = app_props["environment"]["LD_LIBRARY_PATH"] + + if root_ld_library_path is None and not app_environment: + return + + ld_library_path_empty = set() + if root_ld_library_path is None and app_environment: + ld_library_path_empty = { + name + for name, ld_env in app_environment.items() + if "$LD_LIBRARY_PATH" in ld_env + } + elif ( + root_ld_library_path is not None + and "LD_LIBRARY_PATH" in root_ld_library_path + ): + ld_library_path_empty = {"."} + + _EMPTY_LD_LIBRARY_PATH_ITEM_PATTERN = re.compile("^:|::|:$") + + for name, ld_env in app_environment.items(): + if _EMPTY_LD_LIBRARY_PATH_ITEM_PATTERN.findall(ld_env): + ld_library_path_empty.add(name) + + if ( + root_ld_library_path is not None + and _EMPTY_LD_LIBRARY_PATH_ITEM_PATTERN.findall(root_ld_library_path) + ): + ld_library_path_empty.add(".") + + if ld_library_path_empty: + logger.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.".format( + formatting_utils.humanize_list(sorted(ld_library_path_empty), "and") + ) + ) + def generate_hook_wrappers(self) -> None: snap_hooks_dir = os.path.join(self._prime_dir, "snap", "hooks") hooks_dir = os.path.join(self._prime_dir, "meta", "hooks") @@ -518,7 +567,15 @@ if assembled_env: print("{}".format(assembled_env), file=f) print( - "export LD_LIBRARY_PATH=$SNAP_LIBRARY_PATH:$LD_LIBRARY_PATH", file=f + 'export LD_LIBRARY_PATH="$SNAP_LIBRARY_PATH${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}"', + file=f, + ) + print( + 'echo $LD_LIBRARY_PATH | grep -qE "::|^:|:$" && ' + 'echo "WARNING: an empty LD_LIBRARY_PATH has been set. ' + "CWD will be added to the library path. " + 'This can cause the incorrect library to be loaded."', + file=f, ) if cwd: print("{}".format(cwd), file=f) diff -Nru snapcraft-2.43.1/snapcraft/internal/project_loader/_config.py snapcraft-2.43.1+16.04.1/snapcraft/internal/project_loader/_config.py --- snapcraft-2.43.1/snapcraft/internal/project_loader/_config.py 2018-08-30 00:08:05.000000000 +0000 +++ snapcraft-2.43.1+16.04.1/snapcraft/internal/project_loader/_config.py 2020-12-01 12:13:23.000000000 +0000 @@ -297,7 +297,9 @@ if dependency_paths: # Add more specific LD_LIBRARY_PATH from the dependencies. env.append( - 'LD_LIBRARY_PATH="' + ":".join(dependency_paths) + ':$LD_LIBRARY_PATH"' + 'LD_LIBRARY_PATH="' + + ":".join(dependency_paths) + + '${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}"' ) return env diff -Nru snapcraft-2.43.1/snapcraft/internal/project_loader/_env.py snapcraft-2.43.1+16.04.1/snapcraft/internal/project_loader/_env.py --- snapcraft-2.43.1/snapcraft/internal/project_loader/_env.py 2018-08-30 00:08:05.000000000 +0000 +++ snapcraft-2.43.1+16.04.1/snapcraft/internal/project_loader/_env.py 2020-12-01 12:13:23.000000000 +0000 @@ -40,10 +40,8 @@ env.append( 'PATH="' - + ":".join( - ["{0}/usr/sbin", "{0}/usr/bin", "{0}/sbin", "{0}/bin", "$PATH"] - ).format(root) - + '"' + + ":".join(["{0}/usr/sbin", "{0}/usr/bin", "{0}/sbin", "{0}/bin"]).format(root) + + '${PATH:+:$PATH}"' ) # Add the default LD_LIBRARY_PATH diff -Nru snapcraft-2.43.1/tests/unit/project_loader/test_environment.py snapcraft-2.43.1+16.04.1/tests/unit/project_loader/test_environment.py --- snapcraft-2.43.1/tests/unit/project_loader/test_environment.py 2018-08-30 00:08:05.000000000 +0000 +++ snapcraft-2.43.1+16.04.1/tests/unit/project_loader/test_environment.py 2020-12-01 12:13:23.000000000 +0000 @@ -26,9 +26,10 @@ import snapcraft from snapcraft.internal import common -from . import ProjectLoaderBaseTest from tests.fixture_setup.os_release import FakeOsRelease +from . import ProjectLoaderBaseTest + class EnvironmentTest(ProjectLoaderBaseTest): def setUp(self): @@ -55,10 +56,6 @@ lib_paths = [ os.path.join(self.prime_dir, "lib"), os.path.join(self.prime_dir, "usr", "lib"), - os.path.join(self.prime_dir, "lib", project_config.project.arch_triplet), - os.path.join( - self.prime_dir, "usr", "lib", project_config.project.arch_triplet - ), ] for lib_path in lib_paths: os.makedirs(lib_path) @@ -67,43 +64,25 @@ self.assertThat( environment, Contains( - 'PATH="{0}/usr/sbin:{0}/usr/bin:{0}/sbin:{0}/bin:$PATH"'.format( + 'PATH="{0}/usr/sbin:{0}/usr/bin:{0}/sbin:{0}/bin${{PATH:+:$PATH}}"'.format( self.prime_dir ) ), ) - - # Ensure that LD_LIBRARY_PATH is present and it contains only the - # basics. - paths = [] - for variable in environment: - if "LD_LIBRARY_PATH" in variable: - these_paths = variable.split("=")[1].strip() - paths.extend(these_paths.replace('"', "").split(":")) - - self.assertTrue(len(paths) > 0, "Expected LD_LIBRARY_PATH to be in environment") - - expected = ( - os.path.join(self.prime_dir, i) - for i in [ - "lib", - os.path.join("usr", "lib"), - os.path.join("lib", project_config.project.arch_triplet), - os.path.join("usr", "lib", project_config.project.arch_triplet), - ] + self.assertThat( + environment, + Contains( + 'LD_LIBRARY_PATH="${{LD_LIBRARY_PATH:+$LD_LIBRARY_PATH:}}' + '{0}/lib:{0}/usr/lib"'.format(self.prime_dir) + ), ) - for item in expected: - self.assertTrue( - item in paths, - "Expected LD_LIBRARY_PATH in {!r} to include {!r}".format(paths, item), - ) def test_config_snap_environment_with_no_library_paths(self): project_config = self.make_snapcraft_project(self.snapcraft_yaml) environment = project_config.snap_env() self.assertTrue( - 'PATH="{0}/usr/sbin:{0}/usr/bin:{0}/sbin:{0}/bin:$PATH"'.format( + 'PATH="{0}/usr/sbin:{0}/usr/bin:{0}/sbin:{0}/bin${{PATH:+:$PATH}}"'.format( self.prime_dir ) in environment, @@ -130,20 +109,14 @@ # Ensure that LD_LIBRARY_PATH is present and it contains the # extra dependency paths. - paths = [] - for variable in project_config.snap_env(): - if "LD_LIBRARY_PATH" in variable: - these_paths = variable.split("=")[1].strip() - paths.extend(these_paths.replace('"', "").split(":")) - - self.assertTrue(len(paths) > 0, "Expected LD_LIBRARY_PATH to be in environment") - - expected = (os.path.join(self.prime_dir, i) for i in ["lib1", "lib2"]) - for item in expected: - self.assertTrue( - item in paths, - "Expected LD_LIBRARY_PATH ({!r}) to include {!r}".format(paths, item), - ) + self.assertThat( + project_config.snap_env(), + Contains( + 'LD_LIBRARY_PATH="{0}/lib1:{0}/lib2${{LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}}"'.format( + self.prime_dir + ) + ), + ) @mock.patch.object( snapcraft.internal.pluginhandler.PluginHandler, "get_primed_dependency_paths" @@ -238,13 +211,13 @@ 'PATH="{0}/parts/main/install/usr/sbin:' "{0}/parts/main/install/usr/bin:" "{0}/parts/main/install/sbin:" - '{0}/parts/main/install/bin:$PATH"' + '{0}/parts/main/install/bin${{PATH:+:$PATH}}"' ).format(self.path), ( 'PATH="{0}/stage/usr/sbin:' "{0}/stage/usr/bin:" "{0}/stage/sbin:" - '{0}/stage/bin:$PATH"' + '{0}/stage/bin${{PATH:+:$PATH}}"' ).format(self.path), 'PERL5LIB="{0}/stage/usr/share/perl5/"'.format(self.path), 'SNAPCRAFT_ARCH_TRIPLET="{}"'.format( @@ -286,7 +259,7 @@ part = project_config.parts.get_part("part1") environment = project_config.parts.build_env_for_part(part, root_part=True) self.assertIn( - 'LD_LIBRARY_PATH="$LD_LIBRARY_PATH:{base_core_path}/lib:' + 'LD_LIBRARY_PATH="${{LD_LIBRARY_PATH:+$LD_LIBRARY_PATH:}}{base_core_path}/lib:' "{base_core_path}/usr/lib:{base_core_path}/lib/{arch_triplet}:" '{base_core_path}/usr/lib/{arch_triplet}"'.format( base_core_path=self.base_environment.core_path, @@ -357,42 +330,39 @@ project_config = self.make_snapcraft_project(self.snapcraft_yaml) environment = project_config.stage_env() - self.assertTrue( - 'PATH="{0}/usr/sbin:{0}/usr/bin:{0}/sbin:{0}/bin:$PATH"'.format( + self.assertIn( + 'PATH="{0}/usr/sbin:{0}/usr/bin:{0}/sbin:{0}/bin${{PATH:+:$PATH}}"'.format( self.stage_dir - ) - in environment + ), + environment, ) - self.assertTrue( - 'LD_LIBRARY_PATH="$LD_LIBRARY_PATH:{stage_dir}/lib:' + self.assertIn( + 'LD_LIBRARY_PATH="${{LD_LIBRARY_PATH:+$LD_LIBRARY_PATH:}}{stage_dir}/lib:' "{stage_dir}/usr/lib:{stage_dir}/lib/{arch_triplet}:" '{stage_dir}/usr/lib/{arch_triplet}"'.format( stage_dir=self.stage_dir, arch_triplet=project_config.project.arch_triplet, - ) - in environment, - "Current environment is {!r}".format(environment), + ), + environment, ) - self.assertTrue( + self.assertIn( 'CFLAGS="$CFLAGS -I{stage_dir}/include -I{stage_dir}/usr/include ' "-I{stage_dir}/include/{arch_triplet} " '-I{stage_dir}/usr/include/{arch_triplet}"'.format( stage_dir=self.stage_dir, arch_triplet=project_config.project.arch_triplet, - ) - in environment, - "Current environment is {!r}".format(environment), + ), + environment, ) - self.assertTrue( + self.assertIn( 'CPPFLAGS="$CPPFLAGS -I{stage_dir}/include ' "-I{stage_dir}/usr/include " "-I{stage_dir}/include/{arch_triplet} " '-I{stage_dir}/usr/include/{arch_triplet}"'.format( stage_dir=self.stage_dir, arch_triplet=project_config.project.arch_triplet, - ) - in environment, - "Current environment is {!r}".format(environment), + ), + environment, ) self.assertTrue( 'CXXFLAGS="$CXXFLAGS -I{stage_dir}/include ' diff -Nru snapcraft-2.43.1/tests/unit/test_formatting_utils.py snapcraft-2.43.1+16.04.1/tests/unit/test_formatting_utils.py --- snapcraft-2.43.1/tests/unit/test_formatting_utils.py 2018-08-21 15:49:43.000000000 +0000 +++ snapcraft-2.43.1+16.04.1/tests/unit/test_formatting_utils.py 2020-12-01 12:13:23.000000000 +0000 @@ -65,14 +65,14 @@ def test_one_path(self): paths = ["/bin"] output = formatting_utils.format_path_variable("PATH", paths, "/usr", ":") - self.assertThat(output, Equals('PATH="$PATH:/usr/bin"')) + self.assertThat(output, Equals('PATH="${PATH:+$PATH:}/usr/bin"')) def test_two_paths(self): paths = ["/bin", "/sbin"] output = formatting_utils.format_path_variable("PATH", paths, "/usr", ":") - self.assertThat(output, Equals('PATH="$PATH:/usr/bin:/usr/sbin"')) + self.assertThat(output, Equals('PATH="${PATH:+$PATH:}/usr/bin:/usr/sbin"')) def test_two_paths_other_paremeters(self): paths = ["/usr/bin", "/usr/sbin"] output = formatting_utils.format_path_variable("PATH", paths, "", ",") - self.assertThat(output, Equals('PATH="$PATH,/usr/bin,/usr/sbin"')) + self.assertThat(output, Equals('PATH="${PATH:+$PATH,}/usr/bin,/usr/sbin"')) diff -Nru snapcraft-2.43.1/tests/unit/test_meta.py snapcraft-2.43.1+16.04.1/tests/unit/test_meta.py --- snapcraft-2.43.1/tests/unit/test_meta.py 2018-08-21 15:49:43.000000000 +0000 +++ snapcraft-2.43.1+16.04.1/tests/unit/test_meta.py 2020-12-01 12:13:23.000000000 +0000 @@ -1263,8 +1263,12 @@ expected = ( "#!/bin/sh\n" "PATH=$SNAP/usr/bin:$SNAP/bin\n\n" - "export LD_LIBRARY_PATH=$SNAP_LIBRARY_PATH:" - "$LD_LIBRARY_PATH\n" + 'export LD_LIBRARY_PATH="$SNAP_LIBRARY_PATH' + '${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}"\n' + 'echo $LD_LIBRARY_PATH | grep -qE "::|^:|:$" && ' + 'echo "WARNING: an empty LD_LIBRARY_PATH has been set. ' + "CWD will be added to the library path. This can cause the " + 'incorrect library to be loaded."\n' 'exec "$SNAP/test_relexepath" "$@"\n' ) @@ -1316,8 +1320,12 @@ expected = ( "#!/bin/sh\n" "PATH=$SNAP/usr/bin:$SNAP/bin\n\n" - "export LD_LIBRARY_PATH=$SNAP_LIBRARY_PATH:" - "$LD_LIBRARY_PATH\n" + 'export LD_LIBRARY_PATH="$SNAP_LIBRARY_PATH' + '${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}"\n' + 'echo $LD_LIBRARY_PATH | grep -qE "::|^:|:$" && ' + 'echo "WARNING: an empty LD_LIBRARY_PATH has been set. ' + "CWD will be added to the library path. This can cause the " + 'incorrect library to be loaded."\n' 'exec "$SNAP/test_relexepath" "$@"\n' ) self.assertThat(wrapper_path, FileContains(expected)) @@ -1448,6 +1456,164 @@ ) +class TestRootEnvironmentLibraryPathWarnings(CreateBaseTestCase): + def setUp(self): + super().setUp() + + self.config_data["grade"] = "stable" + + self.fake_logger = fixtures.FakeLogger(level=logging.WARNING) + self.useFixture(self.fake_logger) + + def assert_warnings(self): + self.generate_meta_yaml() + + self.assertThat( + self.fake_logger.output, + Contains( + "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." + ), + ) + + def assert_no_warnings(self): + self.generate_meta_yaml() + + self.assertThat( + self.fake_logger.output, + Not( + Contains( + "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." + ) + ), + ) + + def test_root_ld_library_path(self): + self.config_data["environment"] = {"LD_LIBRARY_PATH": "/foo:$LD_LIBRARY_PATH"} + + self.assert_warnings() + + def test_root_ld_library_path_with_colon_at_start(self): + self.config_data["environment"] = {"LD_LIBRARY_PATH": ":/foo:/bar"} + + self.assert_warnings() + + def test_root_ld_library_path_with_colon_at_end(self): + self.config_data["environment"] = {"LD_LIBRARY_PATH": "/foo:/bar:"} + + self.assert_warnings() + + def test_root_ld_library_path_with_colon_at_middle(self): + self.config_data["environment"] = {"LD_LIBRARY_PATH": "/foo::/bar"} + + self.assert_warnings() + + def test_root_ld_library_path_good(self): + self.config_data["environment"] = {"LD_LIBRARY_PATH": "/foo:/bar"} + + self.assert_no_warnings() + + +class TestAppsEnvironmentLibraryPathWarnings(CreateBaseTestCase): + def setUp(self): + super().setUp() + + self.config_data["grade"] = "stable" + self.config_data["apps"] = { + "app1": {"command": "foo"}, + "app2": {"command": "bar"}, + } + + _create_file(os.path.join(self.prime_dir, "foo"), executable=True) + _create_file(os.path.join(self.prime_dir, "bar"), executable=True) + + self.fake_logger = fixtures.FakeLogger(level=logging.WARNING) + self.useFixture(self.fake_logger) + + def assert_warnings_both(self): + self.generate_meta_yaml() + + self.assertThat( + self.fake_logger.output, + Contains( + "CVE-2020-27348: A potentially empty LD_LIBRARY_PATH has been set for environment " + "in 'app1' and 'app2'. The current working directory will be added to the library " + "path if empty. " + "This can cause unexpected libraries to be loaded." + ), + ) + + def assert_warnings_app1(self): + self.generate_meta_yaml() + + self.assertThat( + self.fake_logger.output, + Contains( + "CVE-2020-27348: A potentially empty LD_LIBRARY_PATH has been set for environment " + "in 'app1'. The current working directory will be added to the library " + "path if empty. " + "This can cause unexpected libraries to be loaded." + ), + ) + + def assert_no_warnings(self): + self.generate_meta_yaml() + + self.assertThat( + self.fake_logger.output, + Not( + Contains( + "CVE-2020-27348: A potentially empty LD_LIBRARY_PATH has been set for environment " + "in 'app1' and 'app2'. The current working directory will be added to the library " + "path if empty. " + "This can cause unexpected libraries to be loaded." + ) + ), + ) + + def test_app_ld_library_path_app1(self): + self.config_data["apps"]["app1"]["environment"] = { + "LD_LIBRARY_PATH": "/foo:$LD_LIBRARY_PATH" + } + + self.assert_warnings_app1() + + def test_app_ld_library_path_app1_app2(self): + self.config_data["apps"]["app1"]["environment"] = { + "LD_LIBRARY_PATH": "/foo:$LD_LIBRARY_PATH" + } + self.config_data["apps"]["app2"]["environment"] = { + "LD_LIBRARY_PATH": "/foo:$LD_LIBRARY_PATH" + } + + self.assert_warnings_both() + + def test_root_ld_library_path_set(self): + self.config_data["environment"] = {"LD_LIBRARY_PATH": "/foo:/bar"} + + self.config_data["apps"]["app1"]["environment"] = { + "LD_LIBRARY_PATH": "/foo:$LD_LIBRARY_PATH" + } + self.config_data["apps"]["app2"]["environment"] = { + "LD_LIBRARY_PATH": "/foo:$LD_LIBRARY_PATH" + } + + self.assert_no_warnings() + + def test_app_ld_library_path_colon_middle_app1_app2(self): + self.config_data["apps"]["app1"]["environment"] = { + "LD_LIBRARY_PATH": "/foo::/bar" + } + self.config_data["apps"]["app2"]["environment"] = { + "LD_LIBRARY_PATH": "/foo::/bar" + } + + self.assert_warnings_both() + + def _create_file(path, *, content="", executable=False): basepath = os.path.dirname(path) if basepath: