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.

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

A potential concurrent bug on the Utils.sanitizeJson #4408

Open
NEUZhangy opened this issue Jul 18, 2023 · 3 comments
Open

A potential concurrent bug on the Utils.sanitizeJson #4408

NEUZhangy opened this issue Jul 18, 2023 · 3 comments
Labels

Comments

@NEUZhangy
Copy link

Describe the bug
A potential concurrent bug on the crafted JSON file to sanitizeJson and cause an exception. This is related to
the U+FFFD Unicode replacement character.
To Reproduce

  @Test
  public void testsanitizer() throws Exception {
    String input = "\u0010{'\u0000\u0000'\"\u0000\"{.\ufffd-0X29295909049550970,\n\n0";
    String want = "{\"\\u0000\\u0000\":\"\\u0000\",\"\":{\"0\":-47455995597866469744,\n\n\"0\":null}}";
    String got = Utils.sanitizeJson(input);
    assertEquals(want, got);
  }

Screenshots
If applicable, add screenshots to help explain your problem.
image

@NEUZhangy NEUZhangy added the bug label Jul 18, 2023
@gjvoosten
Copy link
Collaborator

@NEUZhangy Since this happens with the next on an iterator returned by Jackson Databind, I would assume the bug lies there, or in the sanitized JSON returned from the JsonSanitizer.

@NEUZhangy
Copy link
Author

NEUZhangy commented Jul 18, 2023

@NEUZhangy Since this happens with the next on an iterator returned by Jackson Databind, I would assume the bug lies there, or in the sanitized JSON returned from the JsonSanitizer.

Thanks for the reply. When I double-checked the code, I think the root cause is here:

  1. When using the test case I mentioned above, in the first loop, it will run to here:
    objectNode.remove(entry.getKey());
    that remove an entry from the object node;
  2. then, since the for-loop is still using the old reference, it will loop to the entry that code just removed, which is a null reference;
    final Map.Entry<String, JsonNode> entry = entryIter.next();

    that is causing the referenced bug.

@gjvoosten
Copy link
Collaborator

@NEUZhangy Pull requests (preferably against latest HEAD on main) are welcome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants