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)

Conflicts:
	src/rgw/rgw_common.h (trivial)

diff --git a/src/rgw/rgw_auth_s3.cc b/src/rgw/rgw_auth_s3.cc
index 41bd8761cfc327d1c3f0fddf649934a3a856e75c..22b96b5c2d361de6a293b2c0828199a4d6cc25fc 100644 (file)
--- a/src/rgw/rgw_auth_s3.cc
+++ b/src/rgw/rgw_auth_s3.cc
@@ -431,8 +431,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;
   }
@@ -442,8 +443,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 a068f145596718e59ebcf4a3f5c0c31baf42168e..16b0b9a4e61486d68b4ee757bc53be92b11ba282 100644 (file)
--- a/src/rgw/rgw_common.h
+++ b/src/rgw/rgw_common.h
@@ -378,6 +378,7 @@ class RGWHTTPArgs {
 }; // RGWHTTPArgs
 
 const char *rgw_conf_get(const map<string, 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 map<string, string, ltstr_nocase>& conf_map, const char *name, int def_val);
 bool rgw_conf_get_bool(const map<string, string, ltstr_nocase>& conf_map, const char *name, bool def_val);
 
@@ -405,6 +406,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 4e714d097a2e33fa76da3bc02a16566103b76a3c..3e33ffc037fd631f8b69517cb8cc9904e1d4513b 100644 (file)
--- a/src/rgw/rgw_env.cc
+++ b/src/rgw/rgw_env.cc
@@ -50,11 +50,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 4e5770300267f5c95da203eea6416eefc0a0527a..1bea8fa4db00c433d14661dde95a614e7621643b 100644 (file)
--- a/src/rgw/rgw_opa.cc
+++ b/src/rgw/rgw_opa.cc
@@ -39,7 +39,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 fdf9f28a21dd533ec70387911bb9be27eae3a113..f8874ec1c84876ddf79b66f60dc73142221a391b 100644 (file)
--- a/src/rgw/rgw_rest_s3.cc
+++ b/src/rgw/rgw_rest_s3.cc
@@ -3238,7 +3238,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) {
@@ -5501,8 +5504,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 657fa54ee2007590a0e72a8f5fe93c38f358bc79..00fdec025e195a9aea227229eac0f669e2377f4c 100644 (file)
--- a/src/rgw/rgw_rest_swift.cc
+++ b/src/rgw/rgw_rest_swift.cc
@@ -953,8 +953,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 4c85d82baebcb00f4f9c1cec2e10a0851d3c8246..539e900e941706bb00056e67d99a04f15f19fda1 100644 (file)
--- a/src/rgw/rgw_swift_auth.cc
+++ b/src/rgw/rgw_swift_auth.cc
@@ -664,14 +664,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");
@@ -685,7 +687,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);
     }
   }