Skip to content
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,11 @@
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.TreeMap;
import java.util.logging.Logger;

Expand Down Expand Up @@ -329,21 +331,30 @@ public int getSerializedSize() {
+ " security vulnerability:"
+ " https://github.com/protocolbuffers/protobuf/security/advisories/GHSA-h4h5-3hr4-j3g2";

static void warnPre22Gencode() {
private static final Set<String> loggedPre22TypeNames
= Collections.synchronizedSet(new HashSet<String>());
static void warnPre22Gencode(Class<?> messageClass) {
if (System.getProperty(PRE22_GENCODE_SILENCE_PROPERTY) != null) {
return;
}
UnsupportedOperationException exception =
new UnsupportedOperationException(PRE22_GENCODE_VULNERABILITY_MESSAGE);
String vulnerabilityMessage =
"Vulnerable Protobuf Generated type in use: " + messageName + "\n" +
PRE22_GENCODE_VULNERABILITY_MESSAGE);

if (System.getProperty(PRE22_GENCODE_ERROR_PROPERTY) != null) {
throw exception;
throw new UnsupportedOperationException(vulnerabilityMessage);
}

String messageName = messageClass.getName();
if (!loggedPre22TypeNames.add(messageName)) {
return;
}
logger.warning(exception.toString());
logger.warning(vulnerabilityMessage);
}

/** Used by parsing constructors in generated classes. */
protected void makeExtensionsImmutable() {
warnPre22Gencode();
warnPre22Gencode(getClass());
}

/**
Expand Down Expand Up @@ -933,7 +944,7 @@ protected boolean parseUnknownField(
/** Used by parsing constructors in generated classes. */
@Override
protected void makeExtensionsImmutable() {
warnPre22Gencode();
warnPre22Gencode(getClass());
extensions.makeImmutable();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ protected Object newInstance(UnusedPrivateParameter unused) {
*/
protected void makeExtensionsImmutable() {
// Noop for messages without extensions.
GeneratedMessage.warnPre22Gencode();
GeneratedMessage.warnPre22Gencode(getClass());
}

/**
Expand Down Expand Up @@ -1276,7 +1276,7 @@ protected boolean parseUnknownFieldProto3(
*/
@Override
protected void makeExtensionsImmutable() {
GeneratedMessage.warnPre22Gencode();
GeneratedMessage.warnPre22Gencode(getClass());
extensions.makeImmutable();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2015,39 +2015,58 @@ private TestUtil.TestLogHandler setupLogger() {
@Test
public void generatedMessage_makeExtensionsImmutableShouldLog() {
TestUtil.TestLogHandler logHandler = setupLogger();
GeneratedMessageV3 msg =
new GeneratedMessageV3() {
@Override
protected FieldAccessorTable internalGetFieldAccessorTable() {
return null;
}

@Override
protected Message.Builder newBuilderForType(BuilderParent parent) {
return null;
}
class TestMessage1 extends GeneratedMessageV3 {
@Override
protected FieldAccessorTable internalGetFieldAccessorTable() {
return null;
}

@Override
protected Message.Builder newBuilderForType(BuilderParent parent) {
return null;
}

@Override
public Message.Builder newBuilderForType() {
return null;
}

@Override
public Message.Builder toBuilder() {
return null;
}

@Override
public Message getDefaultInstanceForType() {
return null;
}
}

@Override
public Message.Builder newBuilderForType() {
return null;
}
class TestMessage2 extends TestMessage1 {}

@Override
public Message.Builder toBuilder() {
return null;
}

@Override
public Message getDefaultInstanceForType() {
return null;
}
};
TestMessage1 msg = new TestMessage1();
msg.makeExtensionsImmutable();
List<LogRecord> logs = logHandler.getStoredLogRecords();
assertThat(logs).hasSize(1);
String message = logs.get(0).getMessage();
// The generated type
assertThat(message).contains(
"Vulnerable Protobuf Generated type in use: " +
"com.google.protobuf.GeneratedMessageTest$1TestMessage1");
assertThat(message).contains(GeneratedMessage.PRE22_GENCODE_VULNERABILITY_MESSAGE);
assertThat(message).contains(GeneratedMessage.PRE22_GENCODE_SILENCE_PROPERTY);

// Subsequent calls for the same type do not log again.
msg.makeExtensionsImmutable();
assertThat(logHandler.getStoredLogRecords()).hasSize(1);

// A call on a second type does log for that type.
TestMessage2 msg2 = new TestMessage2();
msg2.makeExtensionsImmutable();
assertThat(logHandler.getStoredLogRecords()).hasSize(2);
// And not again (only once per type).
msg2.makeExtensionsImmutable();
assertThat(logHandler.getStoredLogRecords()).hasSize(2);
}

@Test
Expand All @@ -2059,7 +2078,12 @@ public void extendableMessage_makeExtensionsImmutableShouldThrow() {
List<LogRecord> logs = logHandler.getStoredLogRecords();
assertThat(logs).hasSize(1);
String message = logs.get(0).getMessage();
assertThat(message).contains("Vulnerable Protobuf Generated type in use: protobuf_unittest.UnittestProto$TestAllExtensions");
assertThat(message).contains(GeneratedMessage.PRE22_GENCODE_VULNERABILITY_MESSAGE);
assertThat(message).contains(GeneratedMessage.PRE22_GENCODE_SILENCE_PROPERTY);

// Subsequent calls for the same type do not log again.
msg.makeExtensionsImmutable();
assertThat(logHandler.getStoredLogRecords()).hasSize(1);
}
}
Loading