From 1f780a712a4a819cf9b93c47af4bc25d32dedea4 Mon Sep 17 00:00:00 2001 From: Mohamed Awnallah Date: Wed, 1 Mar 2023 18:59:33 +0200 Subject: [PATCH] rgw: Fix potential null pointer dereferences where `RGWEnv::get()` is called Here are the changes I've made: - Added `RGWEnv::get_optional` with the similar implementation to the `RGWHTTPArgs::get_optional` - Replaced `RGWEnv::get` in the RGW code where null pointer derefence happens with `RGWEnv::get_optional` as long as it accepts `std::string` - Otherwise if calling function of `RGWEnv::get` accepts `char*` I leave it as it is - Added null pointer checks to avoid the null pointer dereference This commit addresses the following Coverity CIDs: - "1510310" - "1510928" - "1511097" - "1511555" - "1511760" - "1511951" - "1512003" - "1512138" - "1512398" Fixes: https://tracker.ceph.com/issues/57347 Signed-off-by: Mohamed Awnallah (cherry picked from commit b8cb558cc35d9fb5c47372a79c6249207a964245) --- src/rgw/rgw_auth_s3.cc | 8 +++++--- src/rgw/rgw_common.h | 3 +++ src/rgw/rgw_env.cc | 15 +++++++++++++++ src/rgw/rgw_opa.cc | 5 ++++- src/rgw/rgw_rest_s3.cc | 10 +++++++--- src/rgw/rgw_rest_swift.cc | 5 +++-- src/rgw/rgw_swift_auth.cc | 12 +++++++----- 7 files changed, 44 insertions(+), 14 deletions(-) diff --git a/src/rgw/rgw_auth_s3.cc b/src/rgw/rgw_auth_s3.cc index be2c430286199..241d14f6efe80 100644 --- a/src/rgw/rgw_auth_s3.cc +++ b/src/rgw/rgw_auth_s3.cc @@ -434,8 +434,9 @@ static inline int parse_v4_auth_header(const req_info& info, /* in /* grab date */ const char *d = info.env->get("HTTP_X_AMZ_DATE"); + struct tm t; - if (!parse_iso8601(d, &t, NULL, false)) { + if (!d || !parse_iso8601(d, &t, NULL, false)) { ldpp_dout(dpp, 10) << "error reading date via http_x_amz_date" << dendl; return -EACCES; } @@ -445,8 +446,9 @@ static inline int parse_v4_auth_header(const req_info& info, /* in return -ERR_REQUEST_TIME_SKEWED; } - if (info.env->exists("HTTP_X_AMZ_SECURITY_TOKEN")) { - sessiontoken = info.env->get("HTTP_X_AMZ_SECURITY_TOKEN"); + auto token = info.env->get_optional("HTTP_X_AMZ_SECURITY_TOKEN"); + if (token) { + sessiontoken = *token; } return 0; diff --git a/src/rgw/rgw_common.h b/src/rgw/rgw_common.h index 25e5b24ad8170..8e778b56dd62b 100644 --- a/src/rgw/rgw_common.h +++ b/src/rgw/rgw_common.h @@ -410,6 +410,7 @@ class RGWHTTPArgs { }; // RGWHTTPArgs const char *rgw_conf_get(const std::map& conf_map, const char *name, const char *def_val); +boost::optional rgw_conf_get_optional(const std::map& conf_map, const std::string& name); int rgw_conf_get_int(const std::map& conf_map, const char *name, int def_val); bool rgw_conf_get_bool(const std::map& conf_map, const char *name, bool def_val); @@ -437,6 +438,8 @@ public: void init(CephContext *cct, char **envp); void set(std::string name, std::string val); const char *get(const char *name, const char *def_val = nullptr) const; + boost::optional + get_optional(const std::string& name) const; int get_int(const char *name, int def_val = 0) const; bool get_bool(const char *name, bool def_val = 0); size_t get_size(const char *name, size_t def_val = 0) const; diff --git a/src/rgw/rgw_env.cc b/src/rgw/rgw_env.cc index bb45ee8d36aa1..d528f0e6d479a 100644 --- a/src/rgw/rgw_env.cc +++ b/src/rgw/rgw_env.cc @@ -52,11 +52,26 @@ const char *rgw_conf_get(const map& conf_map, cons return iter->second.c_str(); } +boost::optional rgw_conf_get_optional(const map& conf_map, const std::string& name) +{ + auto iter = conf_map.find(name); + if (iter == conf_map.end()) + return boost::none; + + return boost::optional(iter->second); +} + const char *RGWEnv::get(const char *name, const char *def_val) const { return rgw_conf_get(env_map, name, def_val); } +boost::optional +RGWEnv::get_optional(const std::string& name) const +{ + return rgw_conf_get_optional(env_map, name); +} + int rgw_conf_get_int(const map& conf_map, const char *name, int def_val) { auto iter = conf_map.find(name); diff --git a/src/rgw/rgw_opa.cc b/src/rgw/rgw_opa.cc index dfd3f4f8e1bd2..7422615aec908 100644 --- a/src/rgw/rgw_opa.cc +++ b/src/rgw/rgw_opa.cc @@ -42,7 +42,10 @@ int rgw_opa_authorize(RGWOp *& op, JSONFormatter jf; jf.open_object_section(""); jf.open_object_section("input"); - jf.dump_string("method", s->info.env->get("REQUEST_METHOD")); + const char *request_method = s->info.env->get("REQUEST_METHOD"); + if (request_method) { + jf.dump_string("method", request_method); + } jf.dump_string("relative_uri", s->relative_uri.c_str()); jf.dump_string("decoded_uri", s->decoded_uri.c_str()); jf.dump_string("params", s->info.request_params.c_str()); diff --git a/src/rgw/rgw_rest_s3.cc b/src/rgw/rgw_rest_s3.cc index 720a6cc10e497..6463a34428ddb 100644 --- a/src/rgw/rgw_rest_s3.cc +++ b/src/rgw/rgw_rest_s3.cc @@ -3394,7 +3394,10 @@ int RGWCopyObj_ObjStore_S3::get_params(optional_yield y) s->info.args.get_bool(RGW_SYS_PARAM_PREFIX "copy-if-newer", ©_if_newer, false); } - copy_source = s->info.env->get("HTTP_X_AMZ_COPY_SOURCE"); + const char *copy_source_temp = s->info.env->get("HTTP_X_AMZ_COPY_SOURCE"); + if (copy_source_temp) { + copy_source = copy_source_temp; + } auto tmp_md_d = s->info.env->get("HTTP_X_AMZ_METADATA_DIRECTIVE"); if (tmp_md_d) { if (strcasecmp(tmp_md_d, "COPY") == 0) { @@ -5825,8 +5828,9 @@ AWSGeneralAbstractor::get_auth_data_v2(const req_state* const s) const signature = auth_str.substr(pos + 1); } - if (s->info.env->exists("HTTP_X_AMZ_SECURITY_TOKEN")) { - session_token = s->info.env->get("HTTP_X_AMZ_SECURITY_TOKEN"); + auto token = s->info.env->get_optional("HTTP_X_AMZ_SECURITY_TOKEN"); + if (token) { + session_token = *token; if (session_token.size() == 0) { throw -EPERM; } diff --git a/src/rgw/rgw_rest_swift.cc b/src/rgw/rgw_rest_swift.cc index 119bdc04c8394..1357eae91e152 100644 --- a/src/rgw/rgw_rest_swift.cc +++ b/src/rgw/rgw_rest_swift.cc @@ -945,8 +945,9 @@ int RGWPutObj_ObjStore_SWIFT::get_params(optional_yield y) if (!s->cct->_conf->rgw_swift_custom_header.empty()) { string custom_header = s->cct->_conf->rgw_swift_custom_header; - if (s->info.env->exists(custom_header.c_str())) { - user_data = s->info.env->get(custom_header.c_str()); + auto data = s->info.env->get_optional(custom_header); + if (data) { + user_data = *data; } } diff --git a/src/rgw/rgw_swift_auth.cc b/src/rgw/rgw_swift_auth.cc index 8f41807368c99..594bda55c2e39 100644 --- a/src/rgw/rgw_swift_auth.cc +++ b/src/rgw/rgw_swift_auth.cc @@ -667,14 +667,16 @@ void RGW_SWIFT_Auth_Get::execute(optional_yield y) if (swift_url.size() == 0) { bool add_port = false; - const char *server_port = s->info.env->get("SERVER_PORT_SECURE"); + auto server_port = s->info.env->get_optional("SERVER_PORT_SECURE"); const char *protocol; if (server_port) { - add_port = (strcmp(server_port, "443") != 0); + add_port = (*server_port != "443"); protocol = "https"; } else { - server_port = s->info.env->get("SERVER_PORT"); - add_port = (strcmp(server_port, "80") != 0); + server_port = s->info.env->get_optional("SERVER_PORT"); + if (server_port) { + add_port = (*server_port != "80"); + } protocol = "http"; } const char *host = s->info.env->get("HTTP_HOST"); @@ -688,7 +690,7 @@ void RGW_SWIFT_Auth_Get::execute(optional_yield y) swift_url.append(host); if (add_port && !strchr(host, ':')) { swift_url.append(":"); - swift_url.append(server_port); + swift_url.append(*server_port); } } -- 2.39.5