Gmail Notifier and Gmail Manager extensions evaluate JSON data in an unsafe way

RESOLVED FIXED

Status

RESOLVED FIXED
10 years ago
3 years ago

People

(Reporter: gaubugzilla, Assigned: doronr)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: 'Notifier ok, need to check 'Manager, URL)

(Reporter)

Description

10 years ago
Looking at the file components/nsGMNotifierService.js in the Gmail Notifier extension, I see:

      // get the unread count
      var ts = aData.match(/\["ts",.*\]/gm);
      var myArray = eval(ts[0]);
      this.logItem("  -  ts is " + ts);

That's handling JSON data returned by GMail. The regexp isn't restricting the data in any way and simply calls eval() on everything it finds. Google explicitly doesn't endorse Gmail Notifier, if tomorrow they decide to format the hard drive of every user with that extension - they can do that. At least the default is an HTTPS connection from what I can tell.

There is more in that file, always the same pattern - just search for "eval(". For reference: https://developer.mozilla.org/en/JSON#Using_JSON
(Reporter)

Comment 1

10 years ago
content/gm-notifier/console.xul is questionable as well - it will generate HTML code from log messages and insert it into the document (chrome of course). The log messages are added via logItem() method in components/nsGMNotifierService.js. I didn't verify but I guess that some of the log messages could be manipulated by the website to insert HTML code - and since console.xul doesn't do any escaping something like <img src=d onerror="alert(Components.classes)"> will execute JavaScript in privileged context. Simple HTML entity escaping in console.xul is sufficient to fix that issue. On the other hand, using document.createElement(), element.setAttribute() and element.textContent for the log messages wouldn't be much more complicated yet more reliable.
(Assignee)

Updated

10 years ago
Assignee: nobody → doronr
(Assignee)

Comment 2

10 years ago
You should take a look at the gmail manager as well (if you haven't already) - that extension copied my code and probably has the same issue.
(Reporter)

Comment 3

10 years ago
You are right, Gmail Manager has the same eval() calls in components/gmServiceGmail.js. I was only looking at the Top100 extensions which is why Gmail Manager didn't make my list. I hope you don't mind if I don't create a separate bug for Gmail Manager, it is really exactly the same issue there.
Summary: Gmail Notifier extension evaluates JSON data in an unsafe way → Gmail Notifier and Gmail Manager extensions evaluate JSON data in an unsafe way

Comment 4

10 years ago
(In reply to comment #2)
> You should take a look at the gmail manager as well (if you haven't already) -
> that extension copied my code and probably has the same issue.

I'm sorry but your code wasn't copied. I believe Gmail Manager does quite a bit more than your extension and just because of that doesn't mean your code was copied. By all means continue to believe what you want.

Comment 5

10 years ago
I'll definitely update the code to use the JSON decoder though I'm pretty sure Google is a trusted source and they aren't going to do anything malicious =)

Comment 6

10 years ago
Thanks Todd. Regarding Google, unfortunately, you can never make assumptions of what can happen on a person's individual PC so we need the code to be kept as clean & secure as possible. We appreciate your attention to this.
This should have been split if there are two different authors, but anyway:

Doron: have you fixed GMail Notifier or is this bug still a problem?

Todd: have you fixed GMail Manager or is this bug sill a problem?

If both are fixed we can close this bug. If neither are fixed I'll clone a new bug for GMail Manager. If one is fixed, one not then this bug will suffice and we can update the summary.

(and if I get no response I'll eventually get back around to this and check myself, but please do let me know.)
Component: Administration → Add-on Security
(Assignee)

Comment 8

9 years ago
The gmail notifier switched over to using sandboxes a while ago, so it is clear.
Thanks Doron
Whiteboard: 'Notifier ok, need to check 'Manager

Comment 10

9 years ago
Hey Daniel, the Gmail Manager code has been updated for some time and I apologize for not providing an update about the status here. Thank you.
Fixing based on comment 8 and comment 10
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
(Reporter)

Comment 12

3 years ago
I think that this should be public by now.
Flags: needinfo?(jorge)
Group: client-services-security
Flags: needinfo?(jorge)
You need to log in before you can comment on or make changes to this bug.