Closed Bug 1635835 Opened 10 months ago Closed 7 months ago

Network response JSON is not parsed if leading XSSI-prevention characters like )]}'

Categories

(DevTools :: Netmonitor, enhancement, P3)

75 Branch
enhancement

Tracking

(firefox81 fixed)

RESOLVED FIXED
81 Branch
Tracking Status
firefox81 --- fixed

People

(Reporter: bradw2k, Assigned: dev)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Keywords: good-first-bug)

Attachments

(3 files, 1 obsolete file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:75.0) Gecko/20100101 Firefox/75.0

Steps to reproduce:

  1. Open dev > Network > Response
  2. Get a JSON response with these prepended (security) characters:
    )]}',

Actual results:

The JSON is not parsed and pretty printed as should be, instead the raw text is shown with error message "SyntaxError: JSON.parse: unexpected character at line 1 column 1 of the JSON data"

Expected results:

Should ignore the prepended security characters, and parse and pretty-print the JSON response.
The prepended security characters are a best-practice: https://security.stackexchange.com/questions/110539/how-does-including-a-magic-prefix-to-a-json-response-work-to-prevent-xssi-attack

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Component: Untriaged → Netmonitor
Product: Firefox → DevTools

Thanks for the report!

Yes, agree, we should improve the way we parse JSON responses.

Honza

Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P3

v0 could add workaround for the most common XSSI prevents, like )]}'. Alternatively, we could trim JSON-type responses from the beginning to the first [/{ by default; which might work for more cases.

Could this be a good first bug, Bomsy?

Flags: needinfo?(hmanilla)
Summary: in dev > network tab, JSON is not parsed if leading characters → Network response JSON is not parsed if leading XSSI-prevention characters like )]}'
Flags: needinfo?(hmanilla)
Keywords: good-first-bug

Hi all,
I'm sorry if this is not standard practice, I'm new so I'm not quite sure what that would be!
I'm looking to work on fixing this, is there somewhere where I need to make this known?

Thanks

Flags: needinfo?(odvarko)

Thanks for your interest in working on this.
I'll assign to you.

Let us know if you need any help.

Assignee: nobody → dev
Flags: needinfo?(odvarko)
Attached file testFile.js
Hi, 
I edited the isJSON function to start parsing only from the first JSON-starting character (`[` or `{`). When run outside of the browser, the isJSON function returns the correct JSON without the XSSI-prevention characters. However, the browser still throws the same error. Here's my test file that I used with the edited function attached. The render function seems to take the output from this function as the output to render, but it's still using the raw webserver output.

Hi,
I edited the isJSON function to start parsing only from the first JSON-starting character ([ or {). When run outside of the browser, the isJSON function returns the correct JSON without the XSSI-prevention characters. However, the browser still throws the same error. Here's my test file that I used with the edited function attached. The render function seems to take the output from this function as the output to render, but it's still using the raw webserver output. (bugzilla won't seem to let me upload a file, so I have pasted it in)

function isBase64(payload) {
    try {
      return btoa(atob(payload)) == payload;
    } catch (err) {
      return false;
    }
}

function isJSON(payloadUnclean) {
  let json, error;
  // Start at the first likely JSON character, so that magic XSSI characters can be avoided.
  const firstSquare = payloadUnclean.indexOf("[");
  const firstCurly = payloadUnclean.indexOf("{");
  const first = Math.max(firstCurly, firstSquare);
  let payload = "";
  if (first !== -1) {
    try {
      payload = payloadUnclean.substring(first);
    } catch (err) {
      error = err
    }
  }
  try {
    json = JSON.parse(payload);
  } catch (err) {
    if (isBase64(payload)) {
      try {
        json = JSON.parse(atob(payload));
      } catch (err64) {
        error = err;
      }
    } else {
      error = err;
    }
  }

  // Do not present JSON primitives (e.g. boolean, strings in quotes, numbers)
  // as JSON expandable tree.
  if (!error) {
    if (typeof json !== "object") {
      return {};
    }
  }

  return {
    json,
    error,
  };
}
let { json, error } = isJSON('while(;;))}]{"name": "John","age": 30,"car": null}')
console.log(json)
console.log(error)
{ name: 'John', age: 30, car: null }
undefined

The browser GUI still returns the original string sent from the webserver.

Sorry, forgot to needinfo. Still doesn't make sense to me, even after looking at the code in the frontend.

Flags: needinfo?(hmanilla)

Doy! I realized that the main page JSON parsing fails because it does not use the same isJSON function. The devtools parsing has been fixed, but the first page of where the browser shows the JSON on load is still broken since it's a seperate component. I will file a seperate bug for that.

Sorry for the spam, all!

Flags: needinfo?(hmanilla)
Attached file Fixes Bug 1635835 r=Honza (obsolete) —
Attachment #9153504 - Attachment description: Fixes Bug 1635835 → Parse JSON responses with XSSI-prevention prefixes.
Attachment #9153504 - Attachment description: Parse JSON responses with XSSI-prevention prefixes. → Fixes Bug 1635835
Attachment #9153504 - Attachment description: Fixes Bug 1635835 → Parse JSON responses with XSSI-prevention prefixes
Duplicate of this bug: 1642764
Attachment #9153504 - Attachment description: Parse JSON responses with XSSI-prevention prefixes → Fixes Bug 1635835
Attachment #9153504 - Attachment description: Fixes Bug 1635835 → Fixes Bug 1635835 r=Honza

I filled a foolow up bug 1643696 for renaming isJSON to parseJSON

Honza

Attachment #9158668 - Attachment description: Bug 1635835 - Fix request parsing with XSSI-escapes in requests and responses r=Honza → Bug 1635835 - Fix request parsing with XSSI-escapes in requests and responses
Attachment #9158668 - Attachment description: Bug 1635835 - Fix request parsing with XSSI-escapes in requests and responses → Bug 1635835 - Fix request parsing with XSSI-escapes in requests and responses r=Honza
Attachment #9158668 - Attachment description: Bug 1635835 - Fix request parsing with XSSI-escapes in requests and responses r=Honza → Bug 1635835 - Fix request parsing with XSSI-escapes in requests and responses
Attachment #9158668 - Attachment description: Bug 1635835 - Fix request parsing with XSSI-escapes in requests and responses → Bug 1635835 - Fix request parsing with XSSI-escapes in requests and responses r=Honza,bomsy
Pushed by hmanilla@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4cee9ec9155e
Fix request parsing with XSSI-escapes in requests and responses r=Honza,bomsy
Status: NEW → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → 81 Branch
You need to log in before you can comment on or make changes to this bug.