internal/fuzz/wirefuzz: add test to verify initialization checks
The UnmarshalInitialized flag produced by Unmarshal and Validate are
filters such that must never have false positives (i.e., report a
partial message as initialized) otherwise it is incorrect.
It can tolerate some degree of false negatives (i.e., report an
initialized message as partial), but that leads to significant
performance degradation needing to do the full initialization check.
These should be the exception, not the norm.
Adjust the fuzzer to search for false-negative cases. For now, we only
require that the Unmarshal and Validate report initialized for any
"normalized" messages which we produce by marshaling intermediate
message again. This is to work around a known case where they cannot
determine initialization status if the wire data relies on protobuf's
merge functionality (where two or more partial messages merge
together to form an initialized message).
Change-Id: I6bb6c6594981ca08a92583bae77e5a2d44924af6
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/231577
Reviewed-by: Damien Neil <dneil@google.com>
diff --git a/internal/fuzz/wirefuzz/fuzz.go b/internal/fuzz/wirefuzz/fuzz.go
index 0c08466..f7a9b74 100644
--- a/internal/fuzz/wirefuzz/fuzz.go
+++ b/internal/fuzz/wirefuzz/fuzz.go
@@ -10,6 +10,7 @@
"google.golang.org/protobuf/internal/impl"
"google.golang.org/protobuf/proto"
+ "google.golang.org/protobuf/reflect/protoregistry"
piface "google.golang.org/protobuf/runtime/protoiface"
fuzzpb "google.golang.org/protobuf/internal/testprotos/fuzz"
@@ -17,14 +18,11 @@
// Fuzz is a fuzzer for proto.Marshal and proto.Unmarshal.
func Fuzz(data []byte) (score int) {
+ // Unmarshal and Validate should agree about the validity of the message.
m1 := &fuzzpb.Fuzz{}
- vout, valid := impl.Validate(m1.ProtoReflect().Type(), piface.UnmarshalInput{
- Buf: data,
- })
- vinit := vout.Flags&piface.UnmarshalInitialized != 0
- if err := (proto.UnmarshalOptions{
- AllowPartial: true,
- }).Unmarshal(data, m1); err != nil {
+ mt := m1.ProtoReflect().Type()
+ _, valid := impl.Validate(mt, piface.UnmarshalInput{Buf: data})
+ if err := (proto.UnmarshalOptions{AllowPartial: true}).Unmarshal(data, m1); err != nil {
switch valid {
case impl.ValidationUnknown:
case impl.ValidationInvalid:
@@ -39,22 +37,46 @@
default:
panic("unmarshal ok with validation status: " + valid.String())
}
- if proto.CheckInitialized(m1) != nil && vinit {
- panic("validation reports partial message is initialized")
+
+ // Unmarshal, Validate, and CheckInitialized should agree about initialization.
+ checkInit := proto.CheckInitialized(m1) == nil
+ methods := m1.ProtoReflect().ProtoMethods()
+ in := piface.UnmarshalInput{Message: mt.New(), Resolver: protoregistry.GlobalTypes}
+ if checkInit {
+ // If the message initialized, the both Unmarshal and Validate should
+ // report it as such. False negatives are tolerated, but have a
+ // significant impact on performance. In general, they should always
+ // properly determine initialization for any normalized message,
+ // we produce by re-marshaling the message.
+ in.Buf, _ = proto.Marshal(m1)
+ if out, _ := methods.Unmarshal(in); out.Flags&piface.UnmarshalInitialized == 0 {
+ panic("unmarshal reports initialized message as partial")
+ }
+ if out, _ := impl.Validate(mt, in); out.Flags&piface.UnmarshalInitialized == 0 {
+ panic("validate reports initialized message as partial")
+ }
+ } else {
+ // If the message is partial, then neither Unmarshal nor Validate
+ // should ever report it as such. False positives are unacceptable.
+ in.Buf = data
+ if out, _ := methods.Unmarshal(in); out.Flags&piface.UnmarshalInitialized != 0 {
+ panic("unmarshal reports partial message as initialized")
+ }
+ if out, _ := impl.Validate(mt, in); out.Flags&piface.UnmarshalInitialized != 0 {
+ panic("validate reports partial message as initialized")
+ }
}
- data1, err := proto.MarshalOptions{
- AllowPartial: true,
- }.Marshal(m1)
+
+ // Round-trip Marshal and Unmarshal should produce the same messages.
+ data1, err := proto.MarshalOptions{AllowPartial: !checkInit}.Marshal(m1)
if err != nil {
panic(err)
}
if proto.Size(m1) != len(data1) {
- panic(fmt.Errorf("size does not match output %v", m1))
+ panic(fmt.Errorf("size does not match output: %d != %d", proto.Size(m1), len(data1)))
}
m2 := &fuzzpb.Fuzz{}
- if err := (proto.UnmarshalOptions{
- AllowPartial: true,
- }).Unmarshal(data1, m2); err != nil {
+ if err := (proto.UnmarshalOptions{AllowPartial: !checkInit}).Unmarshal(data1, m2); err != nil {
panic(err)
}
if !proto.Equal(m1, m2) {
