diff options
author | Jan Tattermusch <jtattermusch@users.noreply.github.com> | 2015-11-05 18:41:05 -0800 |
---|---|---|
committer | Jan Tattermusch <jtattermusch@users.noreply.github.com> | 2015-11-05 18:41:05 -0800 |
commit | ffe25c76eac55347d74bef508410f90f01f1db85 (patch) | |
tree | fa9638179b72bf9e763d1d76993f958546869301 /csharp/src/Google.Protobuf | |
parent | 1470ced7ce6f8f5b42e0747ebbea1754db4a3310 (diff) | |
parent | 6fa17e759737e3225c6cc4ba830b921428c50781 (diff) | |
download | protobuf-ffe25c76eac55347d74bef508410f90f01f1db85.tar.gz protobuf-ffe25c76eac55347d74bef508410f90f01f1db85.tar.bz2 protobuf-ffe25c76eac55347d74bef508410f90f01f1db85.zip |
Merge pull request #941 from jskeet/recursion-limit
Add recursion limit handling to JSON parsing.
Diffstat (limited to 'csharp/src/Google.Protobuf')
-rw-r--r-- | csharp/src/Google.Protobuf/InvalidProtocolBufferException.cs | 7 | ||||
-rw-r--r-- | csharp/src/Google.Protobuf/JsonParser.cs | 37 | ||||
-rw-r--r-- | csharp/src/Google.Protobuf/JsonTokenizer.cs | 27 |
3 files changed, 58 insertions, 13 deletions
diff --git a/csharp/src/Google.Protobuf/InvalidProtocolBufferException.cs b/csharp/src/Google.Protobuf/InvalidProtocolBufferException.cs index 01d55395..cacda648 100644 --- a/csharp/src/Google.Protobuf/InvalidProtocolBufferException.cs +++ b/csharp/src/Google.Protobuf/InvalidProtocolBufferException.cs @@ -95,6 +95,13 @@ namespace Google.Protobuf "Use CodedInputStream.SetRecursionLimit() to increase the depth limit.");
}
+ internal static InvalidProtocolBufferException JsonRecursionLimitExceeded()
+ {
+ return new InvalidProtocolBufferException(
+ "Protocol message had too many levels of nesting. May be malicious. " +
+ "Use JsonParser.Settings to increase the depth limit.");
+ }
+
internal static InvalidProtocolBufferException SizeLimitExceeded()
{
return new InvalidProtocolBufferException(
diff --git a/csharp/src/Google.Protobuf/JsonParser.cs b/csharp/src/Google.Protobuf/JsonParser.cs index 85ef1a01..c34f84fb 100644 --- a/csharp/src/Google.Protobuf/JsonParser.cs +++ b/csharp/src/Google.Protobuf/JsonParser.cs @@ -66,13 +66,16 @@ namespace Google.Protobuf private static readonly JsonParser defaultInstance = new JsonParser(Settings.Default); + // TODO: Consider introducing a class containing parse state of the parser, tokenizer and depth. That would simplify these handlers + // and the signatures of various methods. private static readonly Dictionary<string, Action<JsonParser, IMessage, JsonTokenizer>> WellKnownTypeHandlers = new Dictionary<string, Action<JsonParser, IMessage, JsonTokenizer>> { { Timestamp.Descriptor.FullName, (parser, message, tokenizer) => MergeTimestamp(message, tokenizer.Next()) }, { Duration.Descriptor.FullName, (parser, message, tokenizer) => MergeDuration(message, tokenizer.Next()) }, { Value.Descriptor.FullName, (parser, message, tokenizer) => parser.MergeStructValue(message, tokenizer) }, - { ListValue.Descriptor.FullName, (parser, message, tokenizer) => parser.MergeRepeatedField(message, message.Descriptor.Fields[ListValue.ValuesFieldNumber], tokenizer) }, + { ListValue.Descriptor.FullName, (parser, message, tokenizer) => + parser.MergeRepeatedField(message, message.Descriptor.Fields[ListValue.ValuesFieldNumber], tokenizer) }, { Struct.Descriptor.FullName, (parser, message, tokenizer) => parser.MergeStruct(message, tokenizer) }, { FieldMask.Descriptor.FullName, (parser, message, tokenizer) => MergeFieldMask(message, tokenizer.Next()) }, { Int32Value.Descriptor.FullName, MergeWrapperField }, @@ -93,15 +96,11 @@ namespace Google.Protobuf } /// <summary> - /// Returns a formatter using the default settings. /// </summary> + /// Returns a formatter using the default settings. + /// </summary> public static JsonParser Default { get { return defaultInstance; } } -// Currently the settings are unused. -// TODO: When we've implemented Any (and the json spec is finalized), revisit whether they're -// needed at all. -#pragma warning disable 0414 private readonly Settings settings; -#pragma warning restore 0414 /// <summary> /// Creates a new formatted with the given settings. @@ -147,6 +146,10 @@ namespace Google.Protobuf /// </summary> private void Merge(IMessage message, JsonTokenizer tokenizer) { + if (tokenizer.ObjectDepth > settings.RecursionLimit) + { + throw InvalidProtocolBufferException.JsonRecursionLimitExceeded(); + } if (message.Descriptor.IsWellKnownType) { Action<JsonParser, IMessage, JsonTokenizer> handler; @@ -787,14 +790,13 @@ namespace Google.Protobuf #endregion /// <summary> - /// Settings controlling JSON parsing. (Currently doesn't have any actual settings, but I suspect - /// we'll want them for levels of strictness, descriptor pools for Any handling, etc.) + /// Settings controlling JSON parsing. /// </summary> public sealed class Settings { - private static readonly Settings defaultInstance = new Settings(); + private static readonly Settings defaultInstance = new Settings(CodedInputStream.DefaultRecursionLimit); - // TODO: Add recursion limit. + private readonly int recursionLimit; /// <summary> /// Default settings, as used by <see cref="JsonParser.Default"/> @@ -802,10 +804,19 @@ namespace Google.Protobuf public static Settings Default { get { return defaultInstance; } } /// <summary> - /// Creates a new <see cref="Settings"/> object. + /// The maximum depth of messages to parse. Note that this limit only applies to parsing + /// messages, not collections - so a message within a collection within a message only counts as + /// depth 2, not 3. + /// </summary> + public int RecursionLimit { get { return recursionLimit; } } + + /// <summary> + /// Creates a new <see cref="Settings"/> object with the specified recursion limit. /// </summary> - public Settings() + /// <param name="recursionLimit">The maximum depth of messages to parse</param> + public Settings(int recursionLimit) { + this.recursionLimit = recursionLimit; } } } diff --git a/csharp/src/Google.Protobuf/JsonTokenizer.cs b/csharp/src/Google.Protobuf/JsonTokenizer.cs index 5ed1e449..6589427a 100644 --- a/csharp/src/Google.Protobuf/JsonTokenizer.cs +++ b/csharp/src/Google.Protobuf/JsonTokenizer.cs @@ -58,6 +58,13 @@ namespace Google.Protobuf private readonly PushBackReader reader; private JsonToken bufferedToken; private State state; + private int objectDepth = 0; + + /// <summary> + /// Returns the depth of the stack, purely in objects (not collections). + /// Informally, this is the number of remaining unclosed '{' characters we have. + /// </summary> + internal int ObjectDepth { get { return objectDepth; } } internal JsonTokenizer(TextReader reader) { @@ -66,6 +73,8 @@ namespace Google.Protobuf containerStack.Push(ContainerType.Document); } + // TODO: Why do we allow a different token to be pushed back? It might be better to always remember the previous + // token returned, and allow a parameterless Rewind() method (which could only be called once, just like the current PushBack). internal void PushBack(JsonToken token) { if (bufferedToken != null) @@ -73,6 +82,14 @@ namespace Google.Protobuf throw new InvalidOperationException("Can't push back twice"); } bufferedToken = token; + if (token.Type == JsonToken.TokenType.StartObject) + { + objectDepth--; + } + else if (token.Type == JsonToken.TokenType.EndObject) + { + objectDepth++; + } } /// <summary> @@ -95,6 +112,14 @@ namespace Google.Protobuf { var ret = bufferedToken; bufferedToken = null; + if (ret.Type == JsonToken.TokenType.StartObject) + { + objectDepth++; + } + else if (ret.Type == JsonToken.TokenType.EndObject) + { + objectDepth--; + } return ret; } if (state == State.ReaderExhausted) @@ -142,10 +167,12 @@ namespace Google.Protobuf ValidateState(ValueStates, "Invalid state to read an open brace: "); state = State.ObjectStart; containerStack.Push(ContainerType.Object); + objectDepth++; return JsonToken.StartObject; case '}': ValidateState(State.ObjectAfterProperty | State.ObjectStart, "Invalid state to read a close brace: "); PopContainer(); + objectDepth--; return JsonToken.EndObject; case '[': ValidateState(ValueStates, "Invalid state to read an open square bracket: "); |