Skip to content

Commit

Permalink
Add more validation to integral/floating default values during descri…
Browse files Browse the repository at this point in the history 
…ptor

processing.

protoc already does these validations, but descriptor building should do them
too.
Checks added:
 - Verify that parsing the value consumes the whole string, even in the
 presence of NULL characters.
 - Verify that there are no overflows for integral types.

PiperOrigin-RevId: 707992634
  • Loading branch information
@protobuf-github-bot @copybara-github
protobuf-github-bot authored and copybara-github committed Dec 19, 2024
1 parent 7740bcb commit dad7dc2
Show file tree
Hide file tree
Showing 2 changed files with 265 additions and 14 deletions.
63 changes: 49 additions & 14 deletions src/google/protobuf/descriptor.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <algorithm>
#include <array>
#include <atomic>
#include <cerrno>
#include <cstdint>
#include <cstdlib>
#include <cstring>
Expand Down Expand Up @@ -6591,6 +6592,28 @@ void DescriptorBuilder::CheckFieldJsonNameUniqueness(
}
}

template <typename T>
std::optional<std::string> ParseIntegralDefault(const std::string& value,
T& out) {
static_assert(sizeof(int64_t) <= sizeof(long long)); // NOLINT
static_assert(sizeof(uint64_t) <= sizeof(unsigned long long)); // NOLINT
char* end_pos;
using Large = std::conditional_t<std::is_signed_v<T>, int64_t, uint64_t>;
errno = 0;
Large parsed = std::is_signed_v<T>
? std::strtoll(value.c_str(), &end_pos, 0) // NOLINT
: std::strtoull(value.c_str(), &end_pos, 0); // NOLINT
out = static_cast<T>(parsed);
if (errno == ERANGE || parsed != out ||
(value[0] == '-' && !std::is_signed_v<T>)) {
return absl::StrCat("Value \"", value, "\" out of range.");
}
if (value.empty() || end_pos != &value[value.size()]) {
return absl::StrCat("Couldn't parse default value \"", value, "\".");
}
return absl::nullopt;
}

void DescriptorBuilder::BuildFieldOrExtension(const FieldDescriptorProto& proto,
Descriptor* parent,
FieldDescriptor* result,
Expand Down Expand Up @@ -6667,25 +6690,36 @@ void DescriptorBuilder::BuildFieldOrExtension(const FieldDescriptorProto& proto,
char* end_pos = nullptr;
switch (result->cpp_type()) {
case FieldDescriptor::CPPTYPE_INT32:
result->default_value_int32_t_ =
std::strtol(proto.default_value().c_str(), &end_pos, 0);
if (auto error = ParseIntegralDefault(
proto.default_value(), result->default_value_int32_t_)) {
AddError(result->full_name(), proto,
DescriptorPool::ErrorCollector::DEFAULT_VALUE,
[&] { return *error; });
}
break;
case FieldDescriptor::CPPTYPE_INT64:
static_assert(sizeof(int64_t) == sizeof(long long), // NOLINT
"sizeof int64_t is not sizeof long long");
result->default_value_int64_t_ =
std::strtoll(proto.default_value().c_str(), &end_pos, 0);
if (auto error = ParseIntegralDefault(
proto.default_value(), result->default_value_int64_t_)) {
AddError(result->full_name(), proto,
DescriptorPool::ErrorCollector::DEFAULT_VALUE,
[&] { return *error; });
}
break;
case FieldDescriptor::CPPTYPE_UINT32:
result->default_value_uint32_t_ =
std::strtoul(proto.default_value().c_str(), &end_pos, 0);
if (auto error = ParseIntegralDefault(
proto.default_value(), result->default_value_uint32_t_)) {
AddError(result->full_name(), proto,
DescriptorPool::ErrorCollector::DEFAULT_VALUE,
[&] { return *error; });
}
break;
case FieldDescriptor::CPPTYPE_UINT64:
static_assert(
sizeof(uint64_t) == sizeof(unsigned long long), // NOLINT
"sizeof uint64_t is not sizeof unsigned long long");
result->default_value_uint64_t_ =
std::strtoull(proto.default_value().c_str(), &end_pos, 0);
if (auto error = ParseIntegralDefault(
proto.default_value(), result->default_value_uint64_t_)) {
AddError(result->full_name(), proto,
DescriptorPool::ErrorCollector::DEFAULT_VALUE,
[&] { return *error; });
}
break;
case FieldDescriptor::CPPTYPE_FLOAT:
if (proto.default_value() == "inf") {
Expand Down Expand Up @@ -6760,7 +6794,8 @@ void DescriptorBuilder::BuildFieldOrExtension(const FieldDescriptorProto& proto,
// end_pos is only set non-null by the parsers for numeric types,
// above. This checks that the default was non-empty and had no extra
// junk after the end of the number.
if (proto.default_value().empty() || *end_pos != '\0') {
if (proto.default_value().empty() ||
end_pos != &proto.default_value()[proto.default_value().size()]) {
AddError(result->full_name(), proto,
DescriptorPool::ErrorCollector::DEFAULT_VALUE, [&] {
return absl::StrCat("Couldn't parse default value \"",
Expand Down
216 changes: 216 additions & 0 deletions src/google/protobuf/descriptor_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5079,6 +5079,222 @@ TEST_F(ValidationErrorTest, InvalidDefaults) {
"values.\n");
}

static constexpr absl::string_view kNullChar("\0", 1);

TEST_F(ValidationErrorTest, InvalidDefaultIntegerValues) {
// A null char in the middle
BuildFileWithErrors(
R"pb(
name: "foo.proto"
message_type {
name: "bar"
field {
name: "foo"
number: 1
label: LABEL_OPTIONAL
type: TYPE_INT64
default_value: "1\0"
}
}
)pb",
absl::StrCat(
"foo.proto: bar.foo: DEFAULT_VALUE: Couldn't parse default value \"1",
kNullChar, "\".\n"));

// Other chars after
BuildFileWithErrors(
R"pb(
name: "foo.proto"
message_type {
name: "bar"
field {
name: "foo"
number: 1
label: LABEL_OPTIONAL
type: TYPE_INT64
default_value: "1a"
}
}
)pb",
"foo.proto: bar.foo: DEFAULT_VALUE: Couldn't parse default value "
"\"1a\".\n");

// Overflow of int32_t
BuildFileWithErrors(
R"pb(
name: "foo.proto"
message_type {
name: "bar"
field {
name: "foo"
number: 1
label: LABEL_OPTIONAL
type: TYPE_INT32
default_value: "3000000000"
}
}
)pb",
"foo.proto: bar.foo: DEFAULT_VALUE: Value \"3000000000\" out of "
"range.\n");
BuildFileWithErrors(
R"pb(
name: "foo.proto"
message_type {
name: "bar"
field {
name: "foo"
number: 1
label: LABEL_OPTIONAL
type: TYPE_INT32
default_value: "-3000000000"
}
}
)pb",
"foo.proto: bar.foo: DEFAULT_VALUE: Value \"-3000000000\" out of "
"range.\n");

// Overflow of uint32_t
BuildFileWithErrors(
R"pb(
name: "foo.proto"
message_type {
name: "bar"
field {
name: "foo"
number: 1
label: LABEL_OPTIONAL
type: TYPE_UINT32
default_value: "5000000000"
}
}
)pb",
"foo.proto: bar.foo: DEFAULT_VALUE: Value \"5000000000\" out of "
"range.\n");
BuildFileWithErrors(
R"pb(
name: "foo.proto"
message_type {
name: "bar"
field {
name: "foo"
number: 1
label: LABEL_OPTIONAL
type: TYPE_UINT32
default_value: "-1"
}
}
)pb",
"foo.proto: bar.foo: DEFAULT_VALUE: Value \"-1\" out of range.\n");

// Overflow of int64_t
BuildFileWithErrors(
R"pb(
name: "foo.proto"
message_type {
name: "bar"
field {
name: "foo"
number: 1
label: LABEL_OPTIONAL
type: TYPE_INT64
default_value: "9999999999999999999"
}
}
)pb",
"foo.proto: bar.foo: DEFAULT_VALUE: Value \"9999999999999999999\" out of "
"range.\n");
BuildFileWithErrors(
R"pb(
name: "foo.proto"
message_type {
name: "bar"
field {
name: "foo"
number: 1
label: LABEL_OPTIONAL
type: TYPE_INT64
default_value: "-9999999999999999999"
}
}
)pb",
"foo.proto: bar.foo: DEFAULT_VALUE: Value \"-9999999999999999999\" out "
"of range.\n");

// Overflow of uint64_t
BuildFileWithErrors(
R"pb(
name: "foo.proto"
message_type {
name: "bar"
field {
name: "foo"
number: 1
label: LABEL_OPTIONAL
type: TYPE_UINT64
default_value: "19999999999999999999"
}
}
)pb",
"foo.proto: bar.foo: DEFAULT_VALUE: Value \"19999999999999999999\" out "
"of "
"range.\n");
BuildFileWithErrors(
R"pb(
name: "foo.proto"
message_type {
name: "bar"
field {
name: "foo"
number: 1
label: LABEL_OPTIONAL
type: TYPE_UINT64
default_value: "-1"
}
}
)pb",
"foo.proto: bar.foo: DEFAULT_VALUE: Value \"-1\" out "
"of range.\n");
}

TEST_F(ValidationErrorTest, InvalidDefaultFloatingValues) {
// A null char in the middle
BuildFileWithErrors(
R"pb(
name: "foo.proto"
message_type {
name: "bar"
field {
name: "foo"
number: 1
label: LABEL_OPTIONAL
type: TYPE_FLOAT
default_value: "1\0"
}
}
)pb",
absl::StrCat(
"foo.proto: bar.foo: DEFAULT_VALUE: Couldn't parse default value \"1",
kNullChar, "\".\n"));

// Other chars after
BuildFileWithErrors(
R"pb(
name: "foo.proto"
message_type {
name: "bar"
field {
name: "foo"
number: 1
label: LABEL_OPTIONAL
type: TYPE_DOUBLE
default_value: "1j"
}
}
)pb",
"foo.proto: bar.foo: DEFAULT_VALUE: Couldn't parse default value "
"\"1j\".\n");
}

TEST_F(ValidationErrorTest, NegativeFieldNumber) {
BuildFileWithErrors(
"name: \"foo.proto\" "
Expand Down

0 comments on commit dad7dc2

Please sign in to comment.