Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix oom #2665

Merged
merged 3 commits into from
Mar 17, 2022
Merged

Fix oom #2665

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
72da5d8
Fix oom
shargon Feb 26, 2022
65dfa67
Revert reorder
shargon Mar 9, 2022
79ced35
parameters order
erikzhang Mar 17, 2022
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
58 changes: 32 additions & 26 deletions src/neo/IO/Json/JPathToken.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,67 +122,68 @@ private static JPathToken DequeueToken(Queue<JPathToken> tokens)
public static void ProcessJsonPath(ref JObject[] objects, Queue<JPathToken> tokens)
{
int maxDepth = 6;
int maxObjects = 1024;
while (tokens.Count > 0)
{
JPathToken token = DequeueToken(tokens);
switch (token.Type)
{
case JPathTokenType.Dot:
ProcessDot(ref objects, ref maxDepth, tokens);
ProcessDot(ref objects, ref maxDepth, maxObjects, tokens);
break;
case JPathTokenType.LeftBracket:
ProcessBracket(ref objects, ref maxDepth, tokens);
ProcessBracket(ref objects, ref maxDepth, maxObjects, tokens);
break;
default:
throw new FormatException();
}
}
}

private static void ProcessDot(ref JObject[] objects, ref int maxDepth, Queue<JPathToken> tokens)
private static void ProcessDot(ref JObject[] objects, ref int maxDepth, int maxObjects, Queue<JPathToken> tokens)
{
JPathToken token = DequeueToken(tokens);
switch (token.Type)
{
case JPathTokenType.Asterisk:
Descent(ref objects, ref maxDepth);
Descent(ref objects, ref maxDepth, maxObjects);
break;
case JPathTokenType.Dot:
ProcessRecursiveDescent(ref objects, ref maxDepth, tokens);
ProcessRecursiveDescent(ref objects, ref maxDepth, maxObjects, tokens);
break;
case JPathTokenType.Identifier:
Descent(ref objects, ref maxDepth, token.Content);
Descent(ref objects, ref maxDepth, maxObjects, token.Content);
break;
default:
throw new FormatException();
}
}

private static void ProcessBracket(ref JObject[] objects, ref int maxDepth, Queue<JPathToken> tokens)
private static void ProcessBracket(ref JObject[] objects, ref int maxDepth, int maxObjects, Queue<JPathToken> tokens)
{
JPathToken token = DequeueToken(tokens);
switch (token.Type)
{
case JPathTokenType.Asterisk:
if (DequeueToken(tokens).Type != JPathTokenType.RightBracket)
throw new FormatException();
Descent(ref objects, ref maxDepth);
Descent(ref objects, ref maxDepth, maxObjects);
break;
case JPathTokenType.Colon:
ProcessSlice(ref objects, ref maxDepth, tokens, 0);
ProcessSlice(ref objects, ref maxDepth, maxObjects, tokens, 0);
break;
case JPathTokenType.Number:
JPathToken next = DequeueToken(tokens);
switch (next.Type)
{
case JPathTokenType.Colon:
ProcessSlice(ref objects, ref maxDepth, tokens, int.Parse(token.Content));
ProcessSlice(ref objects, ref maxDepth, maxObjects, tokens, int.Parse(token.Content));
break;
case JPathTokenType.Comma:
ProcessUnion(ref objects, ref maxDepth, tokens, token);
ProcessUnion(ref objects, ref maxDepth, maxObjects, tokens, token);
break;
case JPathTokenType.RightBracket:
Descent(ref objects, ref maxDepth, int.Parse(token.Content));
Descent(ref objects, ref maxDepth, maxObjects, int.Parse(token.Content));
break;
default:
throw new FormatException();
Expand All @@ -193,10 +194,10 @@ private static void ProcessBracket(ref JObject[] objects, ref int maxDepth, Queu
switch (next.Type)
{
case JPathTokenType.Comma:
ProcessUnion(ref objects, ref maxDepth, tokens, token);
ProcessUnion(ref objects, ref maxDepth, maxObjects, tokens, token);
break;
case JPathTokenType.RightBracket:
Descent(ref objects, ref maxDepth, JObject.Parse($"\"{token.Content.Trim('\'')}\"").GetString());
Descent(ref objects, ref maxDepth, maxObjects, JObject.Parse($"\"{token.Content.Trim('\'')}\"").GetString());
break;
default:
throw new FormatException();
Expand All @@ -207,38 +208,39 @@ private static void ProcessBracket(ref JObject[] objects, ref int maxDepth, Queu
}
}

private static void ProcessRecursiveDescent(ref JObject[] objects, ref int maxDepth, Queue<JPathToken> tokens)
private static void ProcessRecursiveDescent(ref JObject[] objects, ref int maxDepth, int maxObjects, Queue<JPathToken> tokens)
{
List<JObject> results = new();
JPathToken token = DequeueToken(tokens);
if (token.Type != JPathTokenType.Identifier) throw new FormatException();
while (objects.Length > 0)
{
results.AddRange(objects.Where(p => p is not null).SelectMany(p => p.Properties).Where(p => p.Key == token.Content).Select(p => p.Value));
Descent(ref objects, ref maxDepth);
Descent(ref objects, ref maxDepth, maxObjects);
if (results.Count > maxObjects) throw new InvalidOperationException(nameof(maxObjects));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest to make the exception more precise, use ArgumentOutOfRangeException or OutOfMemoryException instead.

}
objects = results.ToArray();
}

private static void ProcessSlice(ref JObject[] objects, ref int maxDepth, Queue<JPathToken> tokens, int start)
private static void ProcessSlice(ref JObject[] objects, ref int maxDepth, int maxObjects, Queue<JPathToken> tokens, int start)
{
JPathToken token = DequeueToken(tokens);
switch (token.Type)
{
case JPathTokenType.Number:
if (DequeueToken(tokens).Type != JPathTokenType.RightBracket)
throw new FormatException();
DescentRange(ref objects, ref maxDepth, start, int.Parse(token.Content));
DescentRange(ref objects, ref maxDepth, maxObjects, start, int.Parse(token.Content));
break;
case JPathTokenType.RightBracket:
DescentRange(ref objects, ref maxDepth, start, 0);
DescentRange(ref objects, ref maxDepth, maxObjects, start, 0);
break;
default:
throw new FormatException();
}
}

private static void ProcessUnion(ref JObject[] objects, ref int maxDepth, Queue<JPathToken> tokens, JPathToken first)
private static void ProcessUnion(ref JObject[] objects, ref int maxDepth, int maxObjects, Queue<JPathToken> tokens, JPathToken first)
{
List<JPathToken> items = new() { first };
while (true)
Expand All @@ -255,24 +257,25 @@ private static void ProcessUnion(ref JObject[] objects, ref int maxDepth, Queue<
switch (first.Type)
{
case JPathTokenType.Number:
Descent(ref objects, ref maxDepth, items.Select(p => int.Parse(p.Content)).ToArray());
Descent(ref objects, ref maxDepth, maxObjects, items.Select(p => int.Parse(p.Content)).ToArray());
break;
case JPathTokenType.String:
Descent(ref objects, ref maxDepth, items.Select(p => JObject.Parse($"\"{p.Content.Trim('\'')}\"").GetString()).ToArray());
Descent(ref objects, ref maxDepth, maxObjects, items.Select(p => JObject.Parse($"\"{p.Content.Trim('\'')}\"").GetString()).ToArray());
break;
default:
throw new FormatException();
}
}

private static void Descent(ref JObject[] objects, ref int maxDepth)
private static void Descent(ref JObject[] objects, ref int maxDepth, int maxObjects)
{
if (maxDepth <= 0) throw new InvalidOperationException();
--maxDepth;
objects = objects.Where(p => p is not null).SelectMany(p => p is JArray array ? array : p.Properties.Values).ToArray();
if (objects.Length > maxObjects) throw new InvalidOperationException(nameof(maxObjects));
}

private static void Descent(ref JObject[] objects, ref int maxDepth, params string[] names)
private static void Descent(ref JObject[] objects, ref int maxDepth, int maxObjects, params string[] names)
{
static IEnumerable<JObject> GetProperties(JObject obj, string[] names)
{
Expand All @@ -283,9 +286,10 @@ static IEnumerable<JObject> GetProperties(JObject obj, string[] names)
if (maxDepth <= 0) throw new InvalidOperationException();
--maxDepth;
objects = objects.SelectMany(p => GetProperties(p, names)).ToArray();
if (objects.Length > maxObjects) throw new InvalidOperationException(nameof(maxObjects));
}

private static void Descent(ref JObject[] objects, ref int maxDepth, params int[] indexes)
private static void Descent(ref JObject[] objects, ref int maxDepth, int maxObjects, params int[] indexes)
{
static IEnumerable<JObject> GetElements(JArray array, int[] indexes)
{
Expand All @@ -299,9 +303,10 @@ static IEnumerable<JObject> GetElements(JArray array, int[] indexes)
if (maxDepth <= 0) throw new InvalidOperationException();
--maxDepth;
objects = objects.OfType<JArray>().SelectMany(p => GetElements(p, indexes)).ToArray();
if (objects.Length > maxObjects) throw new InvalidOperationException(nameof(maxObjects));
}

private static void DescentRange(ref JObject[] objects, ref int maxDepth, int start, int end)
private static void DescentRange(ref JObject[] objects, ref int maxDepth, int maxObjects, int start, int end)
{
if (maxDepth <= 0) throw new InvalidOperationException();
--maxDepth;
Expand All @@ -313,6 +318,7 @@ private static void DescentRange(ref JObject[] objects, ref int maxDepth, int st
int count = iEnd - iStart;
return p.Skip(iStart).Take(count);
}).ToArray();
if (objects.Length > maxObjects) throw new InvalidOperationException(nameof(maxObjects));
}
}
}
8 changes: 8 additions & 0 deletions tests/neo.UnitTests/IO/Json/UT_JPath.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Neo.IO.Json;
using System;
using System.Linq;

namespace Neo.UnitTests.IO.Json
{
Expand Down Expand Up @@ -54,6 +55,13 @@ public class UT_JPath
["data"] = null,
};

[TestMethod]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also add a boundary test where the size of an intermediate array is approximately equal to the maximum allowed limit?

The size in case is width^depth (64^6 in this case). So tests for 2^10 (width 10, depth 2) and 32^2 (width 2, depth 32) should pass and result in an array of 1024 empty objects. Note, that the value itself should also be changed in this case (amount of [ should be equal to depth).

This will help to ensure neo-go stays compatible.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad, the second test shouldn't pass because of the depth limit.

public void TestOOM()
{
var filter = "$" + string.Concat(Enumerable.Repeat("[0" + string.Concat(Enumerable.Repeat(",0", 64)) + "]", 6));
Assert.ThrowsException<InvalidOperationException>(() => JObject.Parse("[[[[[[{}]]]]]]").JsonPath(filter));
}

[TestMethod]
public void TestJsonPath()
{
Expand Down