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 <mohamedmohey2352@gmail.com>
(cherry picked from commit b8cb558cc35d9fb5c47372a79c6249207a964245)

diff --git a/src/rgw/rgw_auth_s3.cc b/src/rgw/rgw_auth_s3.cc
index 00d05798533de8068dfe7f3bb2fbef09b26c01d0..960fcb8c4be7b567bf9178a2e2a0f2d0730f52b7 100644 (file)
--- 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 43c0a60af8aa5c554fba6eb55244a12f5c16dae1..648b2e087b491ea0e1b1379c2ca7557af2d09311 100644 (file)
--- a/src/rgw/rgw_common.h
+++ b/src/rgw/rgw_common.h
@@ -424,6 +424,7 @@ class RGWHTTPArgs {
 }; // RGWHTTPArgs
 
 const char *rgw_conf_get(const std::map<std::string, std::string, ltstr_nocase>& conf_map, const char *name, const char *def_val);
+boost::optional<const std::string&> rgw_conf_get_optional(const std::map<std::string, std::string, ltstr_nocase>& conf_map, const std::string& name);
 int rgw_conf_get_int(const std::map<std::string, std::string, ltstr_nocase>& conf_map, const char *name, int def_val);
 bool rgw_conf_get_bool(const std::map<std::string, std::string, ltstr_nocase>& conf_map, const char *name, bool def_val);
 
@@ -451,6 +452,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<const std::string&>
+  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 bb45ee8d36aa18d231f1d30a13db2d0c36421bf9..d528f0e6d479a85b8e8f63897904d2ccf94c5a4d 100644 (file)
--- a/src/rgw/rgw_env.cc
+++ b/src/rgw/rgw_env.cc
@@ -52,11 +52,26 @@ const char *rgw_conf_get(const map<string, string, ltstr_nocase>& conf_map, cons
   return iter->second.c_str();
 }
 
+boost::optional<const std::string&> rgw_conf_get_optional(const map<string, string, ltstr_nocase>& conf_map, const std::string& name)
+{
+  auto iter = conf_map.find(name);
+  if (iter == conf_map.end())
+    return boost::none;
+
+  return boost::optional<const std::string&>(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<const std::string&>
+RGWEnv::get_optional(const std::string& name) const
+{
+  return rgw_conf_get_optional(env_map, name);
+}
+
 int rgw_conf_get_int(const map<string, string, ltstr_nocase>& 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 dfd3f4f8e1bd26060e449ed6ef0b3f0af5e752ac..7422615aec9082882760b26a89ed34946f7fa59d 100644 (file)
--- 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 497afc19d5d96d6ef1441b27b15eeba3bb317c59..c73be0c20ed5e861f0d943daf37c0c373fc6bb33 100644 (file)
--- a/src/rgw/rgw_rest_s3.cc
+++ b/src/rgw/rgw_rest_s3.cc
@@ -3485,7 +3485,10 @@ int RGWCopyObj_ObjStore_S3::get_params(optional_yield y)
     s->info.args.get_bool(RGW_SYS_PARAM_PREFIX "copy-if-newer", &copy_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) {
@@ -5984,8 +5987,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 a9565817005dd37916106b1d4e9deb2dc956bbea..e056d71107f5fe649eb84eba15b4c3bc9e5e11aa 100644 (file)
--- 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 197c4e19dd689d5046cf0e71541e3b27d9aa1eb3..05d4b28c124ee28e18bd95c2032a2cb12622b4a9 100644 (file)
--- 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);
     }
   }