Skip to content

Commit

Permalink
Add bounds checks to response attributes
Browse files Browse the repository at this point in the history
Summary:
The function to write resp attrs has a 65k hard limit.
While we could circumvent this with an alternate method,
lets put in the limit a bit lower.
OK packets also have a hard limit of 16mb, and as we have more
junk thrown in there lets try and not flood it with resp attr stuff.

The API does almost no error checking, so our solution is to truncate
the warnings quietly :). dbug asserts will expose the issue to those
who test thoroughly

Reviewed By: fadimounir

Differential Revision: D28662609

fbshipit-source-id: e14d7379601
  • Loading branch information
abal147 authored and facebook-github-bot committed Jun 2, 2021
1 parent 4c6fbb7 commit a1ad6e1
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 2 deletions.
1 change: 1 addition & 0 deletions sql/protocol_classic.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1296,6 +1296,7 @@ static uchar *net_store_length_fast(uchar *packet, size_t length) {
/* The following will only be used for short strings < 65K */

uchar *net_store_data(uchar *to, const uchar *from, size_t length) {
DBUG_ASSERT(length <= 65535);
to = net_store_length_fast(to, length);
if (length > 0) memcpy(to, from, length);
return to + length;
Expand Down
12 changes: 10 additions & 2 deletions sql/session_tracker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@
#include "sql_string.h"
#include "template_utils.h"

constexpr size_t Session_resp_attr_tracker::MAX_RESP_ATTR_LEN;

static void store_lenenc_string(String &to, const char *from, size_t length);

/**
Expand Down Expand Up @@ -1327,6 +1329,8 @@ bool Session_resp_attr_tracker::store(THD *thd MY_ATTRIBUTE((unused)),

size_t len = net_length_size(attrs_.size());
for (const auto &attr : attrs_) {
DBUG_ASSERT(attr.first.size() <= MAX_RESP_ATTR_LEN);
DBUG_ASSERT(attr.second.size() <= MAX_RESP_ATTR_LEN);
len += net_length_size(attr.first.size()) + attr.first.size();
len += net_length_size(attr.second.size()) + attr.second.size();
}
Expand Down Expand Up @@ -1375,9 +1379,13 @@ void Session_resp_attr_tracker::mark_as_changed(THD *thd MY_ATTRIBUTE((unused)),
LEX_CSTRING *key,
const LEX_CSTRING *value) {
DBUG_ASSERT(key->length > 0);
std::string k(key->str, key->length);
DBUG_ASSERT(key->length <= MAX_RESP_ATTR_LEN);
DBUG_ASSERT(value->length > 0);
DBUG_ASSERT(value->length <= MAX_RESP_ATTR_LEN);

attrs_[k] = std::string(value->str, value->length);
std::string k(key->str, std::min(key->length, MAX_RESP_ATTR_LEN));
std::string val(value->str, std::min(value->length, MAX_RESP_ATTR_LEN));
attrs_[std::move(k)] = std::move(val);
m_changed = true;
}

Expand Down
3 changes: 3 additions & 0 deletions sql/session_tracker.h
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,9 @@ class Session_resp_attr_tracker : public State_tracker {
m_changed = false;
m_enabled = false;
}
// 65535 is the HARD LIMIT. Realistically we should need nothing
// close to this
static constexpr size_t MAX_RESP_ATTR_LEN = 60000;

bool enable(THD *thd) override;
bool check(THD *, set_var *) override { return false; }
Expand Down

0 comments on commit a1ad6e1

Please sign in to comment.