Adar Dembo created KUDU-2622:
--------------------------------
Summary: Validate read and write default value sizes when
deserializing ColumnSchemaPB
Key: KUDU-2622
URL: https://issues.apache.org/jira/browse/KUDU-2622
Project: Kudu
Issue Type: Bug
Components: master, tablet
Affects Versions: 1.8.0
Reporter: Adar Dembo
ColumnSchemaFromPB (see wire_protocol.cc) doesn't validate the size of the
column's read_default or write_default values. Because of this, a specially
crafted ColumnSchemaPB can crash either the master (creating a table or
altering a table to add a column) or the tserver (writing, scanning with a
deprecated ColumnRangePredicate, splitting a key range, and possibly more).
Here's a patch that'll reliably trigger a crash in TSAN:
{noformat}
diff --git a/src/kudu/master/master-test.cc b/src/kudu/master/master-test.cc
index 591644848..7bbc1d160 100644
--- a/src/kudu/master/master-test.cc
+++ b/src/kudu/master/master-test.cc
@@ -787,6 +787,23 @@ TEST_F(MasterTest, TestCreateTableInvalidSchema) {
SecureShortDebugString(resp.error().status()));
}
+TEST_F(MasterTest, TestCreateTableCrashReadDefault) {
+ CreateTableRequestPB req;
+ CreateTableResponsePB resp;
+ RpcController controller;
+
+ req.set_name("table");
+ ColumnSchemaPB* col = req.mutable_schema()->add_columns();
+ col->set_name("col");
+ col->set_type(DECIMAL128);
+ col->set_is_key(true);
+ col->set_read_default_value("foo"); // too short!
+
+ ASSERT_OK(proxy_->CreateTable(req, &resp, &controller));
+ SCOPED_TRACE(SecureDebugString(resp));
+ ASSERT_FALSE(resp.has_error());
+}
+
TEST_F(MasterTest, TestVirtualColumns) {
CreateTableRequestPB req;
CreateTableResponsePB resp;
{noformat}
Two mysteries remain:
# I can't repro the crash anywhere except in TSAN, which makes me think it's
some interaction with libc++'s std::string implementation, and that libstdc++'s
std::string doesn't trigger it for some reason. Though I suspect MSAN would
also catch this, if we supported that.
# The thick clients use validation to prevent users from accidentally doing
this, yet for some reason TestKuduBackup.testRandomBackupAndRestore (in the
kudu-backup project) is able to repro this from time to time. I suspect it's
because it chooses certain values (such as a minimum DECIMAL128 value) that
result in nul bytes in the default value byte sequence, and then server-side
protobuf parses this into a string that terminates at the nul byte (again,
maybe this is due to a libc++ string detail). For example:
{noformat}
TEST_F(MasterTest, TestCreateTableCrashReadDefault) {
CreateTableRequestPB req;
CreateTableResponsePB resp;
RpcController controller;
req.set_name("table");
ColumnSchemaPB* col = req.mutable_schema()->add_columns();
col->set_name("col");
col->set_type(DECIMAL128);
col->set_is_key(true);
string default_val = "\001\000\000_\02231\344=,\377\377\377\377\377\377";
col->set_read_default_value(std::move(default_val));
ASSERT_OK(proxy_->CreateTable(req, &resp, &controller));
SCOPED_TRACE(SecureDebugString(resp));
ASSERT_FALSE(resp.has_error());
}
...
I1108 00:24:17.118602 27003 catalog_manager.cc:1373] Servicing CreateTable
request from {username='adar'} at 127.0.0.1:57014:
name: "table"
schema {
columns {
name: "col"
type: DECIMAL128
is_key: true
read_default_value: "\001"
}
}
Thread 25 "rpc worker-2483" received signal SIGSEGV, Segmentation fault.
0x000000000051ff77 in kudu::Variant::Reset (this=0x7b0c0007c080,
type=kudu::DECIMAL128,
value=0x7b0800037101) at ../../src/kudu/common/types.h:706
706 numeric_.i128 = *static_cast<const int128_t *>(value);
(gdb) up
#1 0x000000000051fc83 in kudu::Variant::Variant (this=0x7b0c0007c080,
type=kudu::UINT8,
value=0x5ce40000025080) at ../../src/kudu/common/types.h:644
644 Reset(type, value);
(gdb) up
#2 0x0000000000518cb5 in kudu::ColumnSchema::ColumnSchema
(this=0x7fffdc3b9650, name=...,
type=kudu::DECIMAL128, is_nullable=false, read_default=0x7b0800037101,
write_default=0x7b0800037121, attributes=..., type_attributes=...)
at ../../src/kudu/common/schema.h:209
209 read_default_(read_default ? new Variant(type, read_default) :
nullptr),
(gdb) up
#3 0x00007ffff333d5bc in kudu::ColumnSchemaFromPB (pb=...)
at ../../src/kudu/common/wire_protocol.cc:291
291 return ColumnSchema(pb.name(), pb.type(), pb.is_nullable(),
(gdb) list
286 attributes.compression = pb.compression();
287 }
288 if (pb.has_cfile_block_size()) {
289 attributes.cfile_block_size = pb.cfile_block_size();
290 }
291 return ColumnSchema(pb.name(), pb.type(), pb.is_nullable(),
292 read_default_ptr, write_default_ptr,
293 attributes, type_attributes);
294 }
295
(gdb) p read_default
$1 = {data_ = 0x7b0800037101 "\001", size_ = 1}
{noformat}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
