1482347 - Eval in specialpowersAPI.js

Status: RESOLVED FIXED

(Core :: DOM: Security, enhancement, P2)

Description

[Vinothkumar Nagasayanan [:vinoth]]

As part of Bug 1473549, we are in the process of adding an assertion to make sure that eval() is not executed with system principal.

Eval() has been used in specialpowersAPI.js, we need few information about whether this eval() can be replaced or prefs need to be added for all tests that are using this specialpowersAPI.js

Comment 1

[Vinothkumar Nagasayanan [:vinoth]]

Hey Geoff Brown,

Please see comment 1. Latest change to specialpowersAPI.js related to eval has been made by you as part of Bug 1367780 (https://hg.mozilla.org/mozilla-central/annotate/4e56a2f51ad739ca52046723448f3129a58f1666/testing/specialpowers/content/specialpowersAPI.js#l41) 

Please let us know whether we can change this file for replacing the eval with someother workaround for getting the global this object. Let me know incase you have any comments on this.

Thanks.

Comment 2

[Geoff Brown [:gbrown]]

I only added the eslint comment on that line. Better to ask :bz, who added the eval in

https://hg.mozilla.org/mozilla-central/rev/322ea4cf5b8e

Comment 3

[Boris Zbarsky [:bzbarsky]]

I think bug 1444973 changed the way browser-test.js so now it's always loaded with a Window on its scope chain.  But it's possible we're still loading specialpowersAPI.js from other non-Window contexts...  And "this" is still not the global when specialpowersAPI.js is loaded from browser-tests.js.

I think there are basically two options here.  One is to explicitly check for each name here and import it if needed:

  try { DOMParser; } catch (e) { Cu.importGlobalProperties(["DOMParser"]); }

and the same for File, InspectorUtils, NodeFilter.  The bareword get is important, because that will throw of the property is not defined, instead of returning undefined.

Another option is to do:

  try {
      Cu.importGlobalProperties(["DOMParser", "File", "InspectorUtils", "NodeFilter"]);
  } catch (e) {
      /* We must be in a Window scope; we already have all those defined. */
  }

or so.

Comment 4

[Hema]

Nice one.
Thank you.
<a href=”http://aktechnoservice.com/services/microwave-oven-service-center-in-hyderabad/”>AK Techno Service </a>

Comment 5

[ravi125]

thanks for sharing these great post
http://aktechnoservice.com/samsung-service-center-in-hyderabad/

Comment 6

[Vinothkumar Nagasayanan [:vinoth]]

Attachment: Bug 1482347 - Usage of Eval in specialpowersAPI.js removed.

Comment 7

[Vinothkumar Nagasayanan [:vinoth]]

Comment on attachment 9004182 [details]
Bug 1482347 - Usage of Eval in specialpowersAPI.js removed.

Hey Jonathan,
Since christoph is on vacation, Please kindly review the patch and let me know if you have any comments.

I have made the changes to specialpowersAPI.js as per comment 3. 
And try server push for this changes are,
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b61da9d33b749a401e01284894762b10465ea386

I am also doubtful whether the few failures in try push are related to this patch changes. Correct me if I am wrong.

Comment 8

[Jonathan Kingston [:jkt] he/him]

Comment on attachment 9004182 [details]
Bug 1482347 - Usage of Eval in specialpowersAPI.js removed.

R+ on phab but as noted there you need a peer also. :)

Comment 9

[Vinothkumar Nagasayanan [:vinoth]]

Comment on attachment 9004182 [details]
Bug 1482347 - Usage of Eval in specialpowersAPI.js removed.

I have made the changes to specialpowersAPI.js as per comment 3. 
And new try server push for this changes are,
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2aeba7c28be33f0008011ee015ac37526e68c894

Please kindly take a look and let me know incase of changes.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 9004182 [details]
Bug 1482347 - Usage of Eval in specialpowersAPI.js removed.

Boris Zbarsky [:bzbarsky, bz on IRC] has approved the revision.

Comment 11

[Jonathan Kingston [:jkt] he/him]

Comment on attachment 9004182 [details]
Bug 1482347 - Usage of Eval in specialpowersAPI.js removed.

Jonathan Kingston [:jkt] has been removed from the revision.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f8fe981dd967
Usage of Eval in specialpowersAPI.js removed. r=bzbarsky

Comment 13

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/f8fe981dd967