Critical Description
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:
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;
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:
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}
