summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--ChangeLog46
-rw-r--r--NEWS6
-rw-r--r--dchroot-dsa/dchroot-dsa-options.cc6
-rw-r--r--dchroot-dsa/dchroot-dsa-session.cc12
-rw-r--r--sbuild/sbuild-chroot-block-device.cc26
-rw-r--r--sbuild/sbuild-chroot-block-device.h18
-rw-r--r--sbuild/sbuild-chroot-config.cc12
-rw-r--r--sbuild/sbuild-chroot-file.cc6
-rw-r--r--sbuild/sbuild-chroot-lvm-snapshot.cc6
-rw-r--r--sbuild/sbuild-chroot-plain.cc6
-rw-r--r--sbuild/sbuild-chroot.cc35
-rw-r--r--sbuild/sbuild-chroot.h23
-rw-r--r--sbuild/sbuild-custom-error.tcc2
-rw-r--r--sbuild/sbuild-keyfile.cc1
-rw-r--r--sbuild/sbuild-session.cc4
-rw-r--r--sbuild/sbuild-session.h1
-rw-r--r--sbuild/sbuild-util.cc9
-rw-r--r--sbuild/sbuild-util.h10
18 files changed, 151 insertions, 78 deletions
diff --git a/ChangeLog b/ChangeLog
index f2889aef..69374904 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,4 +1,48 @@
-2006-06-25 Roger Leigh <rleigh@whinlatter.ukfsn.org>
+2006-06-25 Roger Leigh <rleigh@debian.org>
+
+ * NEWS: Document strict absolute path checks.
+
+ * sbuild/sbuild-chroot-config.cc
+ (parse_data): Catch and report chroot deserialisation errors.
+
+ * sbuild/sbuild-chroot-plain.cc
+ (set_keyfile): "location" must be absolute.
+
+ * sbuild/sbuild-chroot-file.cc
+ (set_keyfile): "file" must be absolute.
+
+ * sbuild/sbuild-chroot.cc
+ (set_keyfile): "mount-device" and "mount-location" must be
+ absolute.
+
+ * sbuild/sbuild-chroot-lvm-snapshot.cc
+ (set_keyfile): "lvm-snapshot-device" must be absolute.
+
+ * sbuild/sbuild-chroot-block-device.cc
+ (get_location): Remove unused function
+ (set_location): Remove unused function
+ (get_keyfile): Remove "location".
+ (set_keyfile): Obsolete "location". "device" must be absolute.
+
+ * sbuild/sbuild-chroot.h (sbuild): Add DEVICE_ABS, FILE_ABS and
+ LOCATION_ABS error codes.
+
+ * sbuild/sbuild-keyfile.cc (check_priority): Add missing break in
+ switch.
+
+ * dchroot-dsa/dchroot-dsa-session.cc (get_user_command): Throw a
+ COMMAND_ABS error if the command is not absolute.
+
+ * sbuild/sbuild-session.(cc|h): Add a COMMAND_ABS error code.
+
+ * dchroot-dsa/dchroot-dsa-options.cc (check_options): Throw an
+ error if the command is not an absolute path. This optimisation
+ means a session is not set up if it is already known it will fail.
+
+ * sbuild/sbuild-util.cc (is_absname): New function. This checks
+ if a path is absolute.
+
+2006-06-25 Roger Leigh <rleigh@debian.org>
* All derived chroot types: Replace print_details with
get_details.
diff --git a/NEWS b/NEWS
index 34945bfc..b6bc3f04 100644
--- a/NEWS
+++ b/NEWS
@@ -24,6 +24,12 @@ configuration.
control over access. Corresponding "source-users" and
"source-root-users" options have been added for source chroots.
+ 4) Files, Devices and Locations in schroot.conf must be absolute
+ pathnames. Relative names are a security risk, because the
+ behaviour may vary depending on the current working directory.
+ It was previously the sysadmin's responsibility to set these
+ correctly, but this rule is now strictly enforced.
+
* Major changes in 0.99.0:
1) In order to support running 32-bit chroots on 64-bit systems, a
diff --git a/dchroot-dsa/dchroot-dsa-options.cc b/dchroot-dsa/dchroot-dsa-options.cc
index e5f04323..fa65f70e 100644
--- a/dchroot-dsa/dchroot-dsa-options.cc
+++ b/dchroot-dsa/dchroot-dsa-options.cc
@@ -21,6 +21,8 @@
#include "dchroot-dsa-options.h"
+#include <sbuild/sbuild-util.h>
+
#include <cstdlib>
#include <iostream>
@@ -100,4 +102,8 @@ options::check_options ()
// dchroot-dsa only allows one command.
if (this->command.size() > 1)
throw opt::validation_error(_("Only one command may be specified"));
+
+ if (!this->command.empty() &&
+ !sbuild::is_absname(this->command[0]))
+ throw opt::validation_error(_("Command must have an absolute path"));
}
diff --git a/dchroot-dsa/dchroot-dsa-session.cc b/dchroot-dsa/dchroot-dsa-session.cc
index 31fe7b14..045bbe65 100644
--- a/dchroot-dsa/dchroot-dsa-session.cc
+++ b/dchroot-dsa/dchroot-dsa-session.cc
@@ -109,16 +109,8 @@ session::get_user_command (sbuild::chroot::ptr& session_chroot,
std::string programstring = command[0];
file = programstring;
- if (file.empty() ||
- (file.size() >= 1 && file[0] != '/'))
- {
- sbuild::log_error()
- << format(_("%1%: Command must have an absolute path"))
- % file
- << endl;
- exit (EXIT_FAILURE);
- }
-
+ if (!sbuild::is_absname(file))
+ throw error(file, COMMAND_ABS);
std::string commandstring = sbuild::string_list_to_string(command, " ");
sbuild::log_debug(sbuild::DEBUG_NOTICE)
diff --git a/sbuild/sbuild-chroot-block-device.cc b/sbuild/sbuild-chroot-block-device.cc
index 1b39205c..7d569115 100644
--- a/sbuild/sbuild-chroot-block-device.cc
+++ b/sbuild/sbuild-chroot-block-device.cc
@@ -84,18 +84,6 @@ chroot_block_device::set_mount_options (std::string const& mount_options)
}
std::string const&
-chroot_block_device::get_location () const
-{
- return chroot::get_location();
-}
-
-void
-chroot_block_device::set_location (std::string const& location)
-{
- chroot::set_location(location);
-}
-
-std::string const&
chroot_block_device::get_chroot_type () const
{
static const std::string type("block-device");
@@ -191,9 +179,6 @@ chroot_block_device::get_keyfile (keyfile& keyfile) const
keyfile.set_value(get_name(), "mount-options",
get_mount_options());
-
- keyfile.set_value(get_name(), "location",
- get_location());
}
void
@@ -204,7 +189,11 @@ chroot_block_device::set_keyfile (keyfile const& keyfile)
std::string device;
if (keyfile.get_value(get_name(), "device",
keyfile::PRIORITY_REQUIRED, device))
- set_device(device);
+ {
+ if (!is_absname(device))
+ throw error(device, DEVICE_ABS);
+ set_device(device);
+ }
std::string mount_options;
if (keyfile.get_value(get_name(), "mount-options",
@@ -212,9 +201,8 @@ chroot_block_device::set_keyfile (keyfile const& keyfile)
set_mount_options(mount_options);
std::string location;
- if (keyfile.get_value(get_name(), "location",
- keyfile::PRIORITY_OPTIONAL, location))
- set_location(location);
+ keyfile.get_value(get_name(), "location",
+ keyfile::PRIORITY_OBSOLETE, location);
}
/*
diff --git a/sbuild/sbuild-chroot-block-device.h b/sbuild/sbuild-chroot-block-device.h
index 2dc3f5be..82564aea 100644
--- a/sbuild/sbuild-chroot-block-device.h
+++ b/sbuild/sbuild-chroot-block-device.h
@@ -82,24 +82,6 @@ namespace sbuild
void
set_mount_options (std::string const& mount_options);
- /**
- * Get the location. This is a path to the chroot directory
- * inside the LV (absolute path from the LV root).
- *
- * @returns the location.
- */
- virtual std::string const&
- get_location () const;
-
- /**
- * Set the location. This is a path to the chroot directory
- * inside the LV (absolute path from the LV root).
- *
- * @param location the location.
- */
- virtual void
- set_location (std::string const& location);
-
virtual std::string const&
get_chroot_type () const;
diff --git a/sbuild/sbuild-chroot-config.cc b/sbuild/sbuild-chroot-config.cc
index db108e30..ac5dfb56 100644
--- a/sbuild/sbuild-chroot-config.cc
+++ b/sbuild/sbuild-chroot-config.cc
@@ -474,7 +474,17 @@ chroot_config::parse_data (std::istream& stream,
kconfig.get_value(*group, "type", type);
chroot::ptr chroot = chroot::create(type);
chroot->set_name(*group);
- kconfig >> chroot;
+
+ try
+ {
+ kconfig >> chroot;
+ }
+ catch (const runtime_error& e)
+ {
+ format fmt(_("%1% chroot"));
+ fmt % *group;
+ throw error(fmt.str(), e.what());
+ }
add(chroot);
diff --git a/sbuild/sbuild-chroot-file.cc b/sbuild/sbuild-chroot-file.cc
index 42d11193..f274ecfb 100644
--- a/sbuild/sbuild-chroot-file.cc
+++ b/sbuild/sbuild-chroot-file.cc
@@ -170,7 +170,11 @@ chroot_file::set_keyfile (keyfile const& keyfile)
std::string file;
if (keyfile.get_value(get_name(), "file",
keyfile::PRIORITY_REQUIRED, file))
- set_file(file);
+ {
+ if (!is_absname(file))
+ throw error(file, FILE_ABS);
+ set_file(file);
+ }
keyfile.get_value(get_name(), "file-repack",
get_active() ?
diff --git a/sbuild/sbuild-chroot-lvm-snapshot.cc b/sbuild/sbuild-chroot-lvm-snapshot.cc
index 193c869f..fd8d611f 100644
--- a/sbuild/sbuild-chroot-lvm-snapshot.cc
+++ b/sbuild/sbuild-chroot-lvm-snapshot.cc
@@ -230,7 +230,11 @@ chroot_lvm_snapshot::set_keyfile (keyfile const& keyfile)
keyfile::PRIORITY_REQUIRED :
keyfile::PRIORITY_DISALLOWED,
snapshot_device))
- set_snapshot_device(snapshot_device);
+ {
+ if (!is_absname(snapshot_device))
+ throw error(snapshot_device, DEVICE_ABS);
+ set_snapshot_device(snapshot_device);
+ }
std::string snapshot_options;
if (keyfile.get_value(get_name(), "lvm-snapshot-options",
diff --git a/sbuild/sbuild-chroot-plain.cc b/sbuild/sbuild-chroot-plain.cc
index dfd17217..70bc07fa 100644
--- a/sbuild/sbuild-chroot-plain.cc
+++ b/sbuild/sbuild-chroot-plain.cc
@@ -136,7 +136,11 @@ chroot_plain::set_keyfile (keyfile const& keyfile)
std::string location;
if (keyfile.get_value(get_name(), "location",
keyfile::PRIORITY_REQUIRED, location))
- set_location(location);
+ {
+ if (!is_absname(location))
+ throw error(location, LOCATION_ABS);
+ set_location(location);
+ }
}
/*
diff --git a/sbuild/sbuild-chroot.cc b/sbuild/sbuild-chroot.cc
index 3f2a155e..db0f98de 100644
--- a/sbuild/sbuild-chroot.cc
+++ b/sbuild/sbuild-chroot.cc
@@ -54,21 +54,24 @@ namespace
*/
emap init_errors[] =
{
- emap(sbuild::chroot::CHROOT_TYPE, N_("Unknown chroot type")),
emap(sbuild::chroot::CHROOT_CREATE, N_("Chroot creation failed")),
emap(sbuild::chroot::CHROOT_DEVICE, N_("Device name not set")),
- emap(sbuild::chroot::SESSION_WRITE, N_("Failed to write session file")),
- emap(sbuild::chroot::SESSION_UNLINK, N_("Failed to unlink session file")),
- emap(sbuild::chroot::FILE_STAT, N_("Failed to stat file")),
+ emap(sbuild::chroot::CHROOT_TYPE, N_("Unknown chroot type")),
+ emap(sbuild::chroot::DEVICE_ABS, N_("Device must have an absolute path")),
+ emap(sbuild::chroot::DEVICE_LOCK, N_("Failed to lock device")),
+ emap(sbuild::chroot::DEVICE_NOTBLOCK, N_("File is not a block device")),
+ emap(sbuild::chroot::DEVICE_STAT, N_("Failed to stat device")),
+ emap(sbuild::chroot::DEVICE_UNLOCK, N_("Failed to unlock device")),
+ emap(sbuild::chroot::FILE_ABS, N_("File must have an absolute path")),
+ emap(sbuild::chroot::FILE_LOCK, N_("Failed to acquire file lock")),
+ emap(sbuild::chroot::FILE_NOTREG, N_("File is not a regular file")),
emap(sbuild::chroot::FILE_OWNER, N_("File is not owned by user root")),
emap(sbuild::chroot::FILE_PERMS, N_("File has write permissions for others")),
- emap(sbuild::chroot::FILE_NOTREG, N_("File is not a regular file")),
- emap(sbuild::chroot::FILE_LOCK, N_("Failed to acquire file lock")),
+ emap(sbuild::chroot::FILE_STAT, N_("Failed to stat file")),
emap(sbuild::chroot::FILE_UNLOCK, N_("Failed to discard file lock")),
- emap(sbuild::chroot::DEVICE_STAT, N_("Failed to stat device")),
- emap(sbuild::chroot::DEVICE_NOTBLOCK, N_("File is not a block device")),
- emap(sbuild::chroot::DEVICE_LOCK, N_("Failed to lock device")),
- emap(sbuild::chroot::DEVICE_UNLOCK, N_("Failed to unlock device"))
+ emap(sbuild::chroot::LOCATION_ABS, N_("Location must have an absolute path")),
+ emap(sbuild::chroot::SESSION_UNLINK, N_("Failed to unlink session file")),
+ emap(sbuild::chroot::SESSION_WRITE, N_("Failed to write session file"))
};
}
@@ -580,14 +583,22 @@ sbuild::chroot::set_keyfile (keyfile const& keyfile)
get_active() ?
keyfile::PRIORITY_REQUIRED : keyfile::PRIORITY_DISALLOWED,
mount_location))
- set_mount_location(mount_location);
+ {
+ if (!is_absname(mount_location))
+ throw error(mount_location, LOCATION_ABS);
+ set_mount_location(mount_location);
+ }
std::string mount_device;
if (keyfile.get_value(this->name, "mount-device",
get_active() ?
keyfile::PRIORITY_OPTIONAL : keyfile::PRIORITY_DISALLOWED,
mount_device))
- set_mount_device(mount_device);
+ {
+ if (!is_absname(mount_device))
+ throw error(mount_device, DEVICE_ABS);
+ set_mount_device(mount_device);
+ }
string_list command_prefix;
if (keyfile.get_list_value(this->name, "command-prefix",
diff --git a/sbuild/sbuild-chroot.h b/sbuild/sbuild-chroot.h
index 8ebe38e7..4414d493 100644
--- a/sbuild/sbuild-chroot.h
+++ b/sbuild/sbuild-chroot.h
@@ -75,21 +75,24 @@ namespace sbuild
/// Error codes.
enum error_code
{
- CHROOT_TYPE, ///< Unknown chroot type.
CHROOT_CREATE, ///< Chroot creation failed.
CHROOT_DEVICE, ///< Chroot device name not set.
- SESSION_WRITE, ///< Failed to write session file.
- SESSION_UNLINK, ///< Failed to unlink session file.
- FILE_STAT, ///< Failed to stat file.
+ CHROOT_TYPE, ///< Unknown chroot type.
+ DEVICE_ABS, ///< Device must have an absolute path.
+ DEVICE_LOCK, ///< Failed to lock device.
+ DEVICE_NOTBLOCK, ///< File is not a block device.
+ DEVICE_STAT, ///< Failed to stat device.
+ DEVICE_UNLOCK, ///< Failed to unlock device.
+ FILE_ABS, ///< File must have an absolute path.
+ FILE_LOCK, ///< Failed to acquire lock.
+ FILE_NOTREG, ///< File is not a regular file.
FILE_OWNER, ///< File is not owned by user root.
FILE_PERMS, ///< File has write permissions for others.
- FILE_NOTREG, ///< File is not a regular file.
- FILE_LOCK, ///< Failed to acquire lock.
+ FILE_STAT, ///< Failed to stat file.
FILE_UNLOCK, ///< Failed to discard lock.
- DEVICE_STAT, ///< Failed to stat device.
- DEVICE_NOTBLOCK, ///< File is not a block device.
- DEVICE_LOCK, ///< Failed to lock device.
- DEVICE_UNLOCK ///< Failed to unlock device.
+ LOCATION_ABS, ///< Location must have an absolute path.
+ SESSION_UNLINK, ///< Failed to unlink session file.
+ SESSION_WRITE ///< Failed to write session file.
};
/// Exception type.
diff --git a/sbuild/sbuild-custom-error.tcc b/sbuild/sbuild-custom-error.tcc
index 37e5ba8b..bfe9bbe5 100644
--- a/sbuild/sbuild-custom-error.tcc
+++ b/sbuild/sbuild-custom-error.tcc
@@ -35,7 +35,7 @@ sbuild::custom_error<T>::get_error (error_type error)
return gettext(pos->second);
// Untranslated: it's a programming error to get this message.
- return "unknown error";
+ return "Unknown error";
}
template <typename T>
diff --git a/sbuild/sbuild-keyfile.cc b/sbuild/sbuild-keyfile.cc
index f1e7a9db..a763e2f9 100644
--- a/sbuild/sbuild-keyfile.cc
+++ b/sbuild/sbuild-keyfile.cc
@@ -373,6 +373,7 @@ keyfile::check_priority (std::string const& group,
<< std::endl;
log_info()
<< _("This option has been removed, and no longer has any effect.") << std::endl;
+ break;
case PRIORITY_DISALLOWED:
{
throw error(group, parse_error::DISALLOWED_KEY, key);
diff --git a/sbuild/sbuild-session.cc b/sbuild/sbuild-session.cc
index 01469e2b..c974bb64 100644
--- a/sbuild/sbuild-session.cc
+++ b/sbuild/sbuild-session.cc
@@ -69,6 +69,7 @@ namespace
emap(session::CHROOT_SETUP, N_("Chroot setup failed")),
emap(session::CHROOT_UNKNOWN, N_("Failed to find chroot")),
emap(session::CHROOT_UNLOCK, N_("Failed to unlock chroot")),
+ emap(session::COMMAND_ABS, N_("Command must have an absolute path")),
emap(session::EXEC, N_("Failed to execute")),
emap(session::GROUP_GET_SUP, N_("Failed to get supplementary groups")),
emap(session::GROUP_GET_SUPC, N_("Failed to get supplementary group count")),
@@ -905,9 +906,6 @@ session::run_child (sbuild::chroot::ptr& session_chroot)
std::string location(session_chroot->get_path());
- /* Child errors result in immediate exit(). Errors are not
- propagated back via an exception, because there is no longer any
- higher-level handler to catch them. */
try
{
open_session();
diff --git a/sbuild/sbuild-session.h b/sbuild/sbuild-session.h
index 048e1294..7e3e29b1 100644
--- a/sbuild/sbuild-session.h
+++ b/sbuild/sbuild-session.h
@@ -75,6 +75,7 @@ namespace sbuild
CHROOT_SETUP, ///< Setup failed.
CHROOT_UNKNOWN, ///< Failed to find chroot.
CHROOT_UNLOCK, ///< Failed to unlock chroot.
+ COMMAND_ABS, ///< Command must have an absolute path.
EXEC, ///< Failed to execute.
GROUP_GET_SUP, ///< Failed to get supplementary groups.
GROUP_GET_SUPC, ///< Failed to get supplementary group count
diff --git a/sbuild/sbuild-util.cc b/sbuild/sbuild-util.cc
index c0b1e141..a6b1ab20 100644
--- a/sbuild/sbuild-util.cc
+++ b/sbuild/sbuild-util.cc
@@ -121,6 +121,15 @@ sbuild::normalname (std::string name,
return remove_duplicates(name, separator);
}
+bool
+sbuild::is_absname (std::string const& name)
+{
+ if (name.empty() || name[0] != '/')
+ return false;
+ else
+ return true;
+}
+
std::string
sbuild::string_list_to_string (sbuild::string_list const& list,
std::string const& separator)
diff --git a/sbuild/sbuild-util.h b/sbuild/sbuild-util.h
index 46522aa2..a849f138 100644
--- a/sbuild/sbuild-util.h
+++ b/sbuild/sbuild-util.h
@@ -64,6 +64,16 @@ namespace sbuild
char separator = '/');
/**
+ * Check if a pathname is absolute.
+ *
+ * @param name the path to check.
+ * @returns true if the name is absolute or false if it is not, or
+ * if name is empty.
+ */
+ bool
+ is_absname (std::string const& name);
+
+ /**
* Convert a string_list into a string. The strings are
* concatenated using separator as a delimiter.
*
generated by cgit v1.2.3 (git 2.39.1) at 2025-02-17 13:46:08 +0000