rgw: sanitize header attributes

Signed-off-by: Yehuda Sadeh <yehuda@redhat.com>

diff --git a/src/rgw/rgw_common.h b/src/rgw/rgw_common.h
index 15348251074979f0fe6a0829c912be8be07f8142..890e746ce32cd0be60857de12a25864045b7be81 100644 (file)
--- a/src/rgw/rgw_common.h
+++ b/src/rgw/rgw_common.h
@@ -2667,4 +2667,15 @@ static inline ssize_t rgw_unescape_str(const string& s, ssize_t ofs,
   return string::npos;
 }
 
+static inline string rgw_bl_str(ceph::buffer::list& raw)
+{
+  size_t len = raw.length();
+  string s(raw.c_str(), len);
+  while (len && !s[len - 1]) {
+    --len;
+    s.resize(len);
+  }
+  return s;
+}
+
 #endif

diff --git a/src/rgw/rgw_rados.cc b/src/rgw/rgw_rados.cc
index 13a6549c48c234f355e9a65afd0b804e804ffc84..43bd8aa230708e7f4bc859ec43dc9488a3336a94 100644 (file)
--- a/src/rgw/rgw_rados.cc
+++ b/src/rgw/rgw_rados.cc
@@ -3622,9 +3622,9 @@ int RGWRados::Object::Write::_do_write_meta(uint64_t size, uint64_t accounted_si
     op.setxattr(name.c_str(), bl);
 
     if (name.compare(RGW_ATTR_ETAG) == 0) {
-      etag = bl.to_str();
+      etag = rgw_bl_str(bl);
     } else if (name.compare(RGW_ATTR_CONTENT_TYPE) == 0) {
-      content_type = bl.to_str();
+      content_type = rgw_bl_str(bl);
     } else if (name.compare(RGW_ATTR_ACL) == 0) {
       acl_bl = bl;
     }
@@ -6206,12 +6206,12 @@ int RGWRados::set_attrs(void *ctx, const RGWBucketInfo& bucket_info, rgw_obj& ob
       bufferlist acl_bl = attrs[RGW_ATTR_ACL];
       bufferlist etag_bl = attrs[RGW_ATTR_ETAG];
       bufferlist content_type_bl = attrs[RGW_ATTR_CONTENT_TYPE];
-      string etag(etag_bl.c_str(), etag_bl.length());
-      string content_type(content_type_bl.c_str(), content_type_bl.length());
+      string etag = rgw_bl_str(etag_bl);
+      string content_type = rgw_bl_str(content_type_bl);
       string storage_class;
       auto iter = attrs.find(RGW_ATTR_STORAGE_CLASS);
       if (iter != attrs.end()) {
-        storage_class = iter->second.to_str();
+        storage_class = rgw_bl_str(iter->second);
       }
       uint64_t epoch = ref.ioctx.get_last_version();
       int64_t poolid = ref.ioctx.get_id();
@@ -9303,11 +9303,11 @@ int RGWRados::check_disk_state(librados::IoCtx io_ctx,
 
   map<string, bufferlist>::iterator iter = astate->attrset.find(RGW_ATTR_ETAG);
   if (iter != astate->attrset.end()) {
-    etag = iter->second.to_str();
+    etag = rgw_bl_str(iter->second);
   }
   iter = astate->attrset.find(RGW_ATTR_CONTENT_TYPE);
   if (iter != astate->attrset.end()) {
-    content_type = iter->second.to_str();
+    content_type = rgw_bl_str(iter->second);
   }
   iter = astate->attrset.find(RGW_ATTR_ACL);
   if (iter != astate->attrset.end()) {

diff --git a/src/rgw/rgw_rest.cc b/src/rgw/rgw_rest.cc
index 3d623c3659ade2ede38b1d6c4f02606c25dc4a09..003d60ff9395fe9ae7357a8012e646505dac7ed5 100644 (file)
--- a/src/rgw/rgw_rest.cc
+++ b/src/rgw/rgw_rest.cc
@@ -335,31 +335,11 @@ void dump_header(struct req_state* const s,
   }
 }
 
-static inline boost::string_ref get_sanitized_hdrval(ceph::buffer::list& raw)
-{
-  /* std::string and thus boost::string_ref ARE OBLIGED to carry multiple
-   * 0x00 and count them to the length of a string. We need to take that
-   * into consideration and sanitize the size of a ceph::buffer::list used
-   * to store metadata values (x-amz-meta-*, X-Container-Meta-*, etags).
-   * Otherwise we might send 0x00 to clients. */
-  const char* const data = raw.c_str();
-  size_t len = raw.length();
-
-  if (len && data[len - 1] == '\0') {
-    /* That's the case - the null byte has been included at the last position
-     * of the bufferlist. We need to restore the proper string length we'll
-     * pass to string_ref. */
-    len--;
-  }
-
-  return boost::string_ref(data, len);
-}
-
 void dump_header(struct req_state* const s,
                  const boost::string_ref& name,
                  ceph::buffer::list& bl)
 {
-  return dump_header(s, name, get_sanitized_hdrval(bl));
+  return dump_header(s, name, rgw_sanitized_hdrval(bl));
 }
 
 void dump_header(struct req_state* const s,

diff --git a/src/rgw/rgw_rest.h b/src/rgw/rgw_rest.h
index e6493bc9d3c8f3bd6a8de1cf813f560453802b16..11d293f93586c0cef0b236e1157d1189cb79660c 100644 (file)
--- a/src/rgw/rgw_rest.h
+++ b/src/rgw/rgw_rest.h
@@ -29,6 +29,26 @@ std::tuple<int, bufferlist > rgw_rest_read_all_input(struct req_state *s,
                                         const uint64_t max_len,
                                         const bool allow_chunked=true);
 
+static inline boost::string_ref rgw_sanitized_hdrval(ceph::buffer::list& raw)
+{
+  /* std::string and thus boost::string_ref ARE OBLIGED to carry multiple
+   * 0x00 and count them to the length of a string. We need to take that
+   * into consideration and sanitize the size of a ceph::buffer::list used
+   * to store metadata values (x-amz-meta-*, X-Container-Meta-*, etags).
+   * Otherwise we might send 0x00 to clients. */
+  const char* const data = raw.c_str();
+  size_t len = raw.length();
+
+  if (len && data[len - 1] == '\0') {
+    /* That's the case - the null byte has been included at the last position
+     * of the bufferlist. We need to restore the proper string length we'll
+     * pass to string_ref. */
+    len--;
+  }
+
+  return boost::string_ref(data, len);
+}
+
 template <class T>
 int rgw_rest_get_json_input(CephContext *cct, req_state *s, T& out,
                            uint64_t max_len, bool *empty)

diff --git a/src/rgw/rgw_rest_s3.cc b/src/rgw/rgw_rest_s3.cc
index c620904b129dcc1a2bbf9b3927c5b2aa520464d0..eab67d128f418387c5fb46df60d1afedbab8e0ca 100644 (file)
--- a/src/rgw/rgw_rest_s3.cc
+++ b/src/rgw/rgw_rest_s3.cc
@@ -291,22 +291,19 @@ int RGWGetObj_ObjStore_S3::send_response_data(bufferlist& bl, off_t bl_ofs,
         if (response_attrs.count(aiter->second) == 0) {
           /* Was not already overridden by a response param. */
 
-          /* clean up attribute, we have cases where we kept extra null character
-           * at the end of the buffer, so bufferlist.to_str() won't work because
-           * it'll generate a string with that extra character
-           */
-          auto& buf = iter->second;
-          const char *val = buf.c_str();
-          size_t len = buf.length();
-          while (len > 0 && !val[len - 1]) {
+          size_t len = iter->second.length();
+          string s(iter->second.c_str(), len);
+          while (len && !s[len - 1]) {
             --len;
+            s.resize(len);
           }
-          response_attrs[aiter->second] = string(val, len);
+          response_attrs[aiter->second] = s;
         }
       } else if (iter->first.compare(RGW_ATTR_CONTENT_TYPE) == 0) {
         /* Special handling for content_type. */
         if (!content_type) {
-          content_type = iter->second.c_str();
+          content_type_str = rgw_bl_str(iter->second);
+          content_type = content_type_str.c_str();
         }
       } else if (strcmp(name, RGW_ATTR_SLO_UINDICATOR) == 0) {
         // this attr has an extra length prefix from encode() in prior versions

diff --git a/src/rgw/rgw_rest_swift.cc b/src/rgw/rgw_rest_swift.cc
index 702a435bd06394aeb87a2860022e2ebb16b1cb1f..e5fab72e3941ede67966900f932039d45e2b00a0 100644 (file)
--- a/src/rgw/rgw_rest_swift.cc
+++ b/src/rgw/rgw_rest_swift.cc
@@ -1312,12 +1312,12 @@ static void get_contype_from_attrs(map<string, bufferlist>& attrs,
 {
   map<string, bufferlist>::iterator iter = attrs.find(RGW_ATTR_CONTENT_TYPE);
   if (iter != attrs.end()) {
-    content_type = iter->second.c_str();
+    content_type = rgw_bl_str(iter->second);
   }
 }
 
 static void dump_object_metadata(struct req_state * const s,
-                                map<string, bufferlist> attrs)
+                                const map<string, bufferlist>& attrs)
 {
   map<string, string> response_attrs;
 
@@ -1326,7 +1326,7 @@ static void dump_object_metadata(struct req_state * const s,
     const auto aiter = rgw_to_http_attrs.find(name);
 
     if (aiter != std::end(rgw_to_http_attrs)) {
-      response_attrs[aiter->second] = kv.second.c_str();
+      response_attrs[aiter->second] = rgw_bl_str(kv.second);
     } else if (strcmp(name, RGW_ATTR_SLO_UINDICATOR) == 0) {
       // this attr has an extra length prefix from encode() in prior versions
       dump_header(s, "X-Object-Meta-Static-Large-Object", "True");