From 4250346b87b8e24a48d04ddacc77512eaa20ce0e Mon Sep 17 00:00:00 2001 From: Emery Hemingway Date: Sat, 7 Nov 2020 11:23:03 +0100 Subject: [PATCH 1/3] base: fail on label truncation --- repos/base/include/base/session_label.h | 27 ++++++++++++++++++++++--- repos/base/include/util/arg_string.h | 6 ++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/repos/base/include/base/session_label.h b/repos/base/include/base/session_label.h index d5e752d120..85034904e5 100644 --- a/repos/base/include/base/session_label.h +++ b/repos/base/include/base/session_label.h @@ -16,10 +16,14 @@ #define _INCLUDE__BASE__SESSION_LABEL_H_ #include +#include #include #include -namespace Genode { struct Session_label; } +namespace Genode { + struct Session_label; + class Label_overflow : Exception { }; +} struct Genode::Session_label : String<160> { @@ -33,6 +37,8 @@ struct Genode::Session_label : String<160> using String = String; using String::String; + /* TODO: String::String can still truncate and break labels */ + /** * Copy constructor * @@ -41,7 +47,13 @@ struct Genode::Session_label : String<160> */ template Session_label(Genode::String const &other) - : Genode::String<160>(other) { } + : Genode::String<160>(other) + { + if (length() < other.length()) { + error(__func__, " overflow - «", other, "»"); + throw Label_overflow(); + } + } Session_label last_element() const { @@ -90,8 +102,13 @@ namespace Genode { inline Session_label label_from_args(char const *args) { char buf[Session_label::capacity()]; - Arg_string::find_arg(args, "label").string(buf, sizeof(buf), ""); + auto arg = Arg_string::find_arg(args, "label"); + if (Session_label::capacity() <= arg.length()) { + error(__func__, " overflow - «", (char const *)args, "»"); + throw Label_overflow(); + } + arg.string(buf, sizeof(buf), ""); return Session_label(Cstring(buf)); } @@ -103,6 +120,10 @@ namespace Genode { String const &label) { String const prefixed_label(prefix, " -> ", label); + if (Session_label::capacity() <= prefixed_label.length()) { + error(__func__, " overflow - «", prefix, "» - «", label, "»"); + throw Label_overflow(); + } return Session_label(prefixed_label); } } diff --git a/repos/base/include/util/arg_string.h b/repos/base/include/util/arg_string.h index 610fbb16b3..48777e0c2a 100644 --- a/repos/base/include/util/arg_string.h +++ b/repos/base/include/util/arg_string.h @@ -114,6 +114,12 @@ class Genode::Arg inline bool valid() const { return _key; } + size_t length() const + { + return _value.type() == Token::STRING + ? _value.len() - 2 : _value.len(); + } + unsigned long ulong_value(unsigned long default_value) const { unsigned long value = 0; -- 2.30.0 From 252c08cf61ad7feef83bd2e542465330633ba41f Mon Sep 17 00:00:00 2001 From: Emery Hemingway Date: Wed, 10 Feb 2021 13:32:42 +0100 Subject: [PATCH 2/3] Detect destroyed argument buffers at Env::session Session request arguments are silently zeroed when their length exceedes some buffer size. --- repos/base/src/lib/base/component.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/repos/base/src/lib/base/component.cc b/repos/base/src/lib/base/component.cc index 568be31efb..913687ea7f 100644 --- a/repos/base/src/lib/base/component.cc +++ b/repos/base/src/lib/base/component.cc @@ -122,6 +122,10 @@ namespace { Affinity const &affinity) override { Mutex::Guard guard(_mutex); + if (!args.valid_string()) { + error("invalid args for ", name.string(), " service request"); + throw Service_denied(); + } /* * Since we account for the backing store for session meta data on -- 2.30.0 From 53641e192bc3f9a756ae15b91640a42ac7e70918 Mon Sep 17 00:00:00 2001 From: Emery Hemingway Date: Thu, 11 Feb 2021 14:10:50 +0100 Subject: [PATCH 3/3] Increase session arguments buffer size to 240 bytes --- repos/base/include/parent/parent.h | 2 +- repos/base/include/root/root.h | 2 +- repos/base/lib/symbols/ld | 2 ++ 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/repos/base/include/parent/parent.h b/repos/base/include/parent/parent.h index 7379342e5b..6e94047788 100644 --- a/repos/base/include/parent/parent.h +++ b/repos/base/include/parent/parent.h @@ -54,7 +54,7 @@ class Genode::Parent public: typedef Rpc_in_buffer<64> Service_name; - typedef Rpc_in_buffer<160> Session_args; + typedef Rpc_in_buffer<240> Session_args; typedef Rpc_in_buffer<160> Upgrade_args; struct Client : Interface { typedef Id_space::Id Id; }; diff --git a/repos/base/include/root/root.h b/repos/base/include/root/root.h index c7e3a0c908..4fda9e341f 100644 --- a/repos/base/include/root/root.h +++ b/repos/base/include/root/root.h @@ -29,7 +29,7 @@ namespace Genode { struct Genode::Root { - typedef Rpc_in_buffer<160> Session_args; + typedef Rpc_in_buffer<240> Session_args; typedef Rpc_in_buffer<160> Upgrade_args; virtual ~Root() { } diff --git a/repos/base/lib/symbols/ld b/repos/base/lib/symbols/ld index 3cfbdd7466..d7603e8ca4 100644 --- a/repos/base/lib/symbols/ld +++ b/repos/base/lib/symbols/ld @@ -398,6 +398,8 @@ _ZThn236_N5Timer10Connection11set_timeoutEN6Genode12MicrosecondsERNS1_15Timeout_ _ZThn236_N5Timer10Connection9curr_timeEv T _ZThn288_N5Timer10Connection11set_timeoutEN6Genode12MicrosecondsERNS1_15Timeout_handlerE T _ZThn288_N5Timer10Connection9curr_timeEv T +_ZThn368_N5Timer10Connection11set_timeoutEN6Genode12MicrosecondsERNS1_15Timeout_handlerE T +_ZThn368_N5Timer10Connection9curr_timeEv T _ZThn8_N6Genode17Timeout_scheduler14handle_timeoutENS_8DurationE T _ZThn8_N6Genode17Timeout_schedulerD0Ev T _ZThn8_N6Genode17Timeout_schedulerD1Ev T -- 2.30.0