From: Yehuda Sadeh Date: Thu, 10 Jan 2019 17:45:42 +0000 (-0800) Subject: rgw: sanitize header attributes X-Git-Tag: v14.1.0~314^2~10 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=dcb5e06ca1d7088b268c7706fe9530e5e99ea579;p=ceph.git rgw: sanitize header attributes Signed-off-by: Yehuda Sadeh --- diff --git a/src/rgw/rgw_common.h b/src/rgw/rgw_common.h index 153482510749..890e746ce32c 100644 --- 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 13a6549c48c2..43bd8aa23070 100644 --- 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::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 3d623c3659ad..003d60ff9395 100644 --- 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 e6493bc9d3c8..11d293f93586 100644 --- a/src/rgw/rgw_rest.h +++ b/src/rgw/rgw_rest.h @@ -29,6 +29,26 @@ std::tuple 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 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 c620904b129d..eab67d128f41 100644 --- 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 702a435bd063..e5fab72e3941 100644 --- 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& attrs, { map::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 attrs) + const map& attrs) { map 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");