Closed Bug 1173228 Opened 5 years ago Closed 5 years ago

[Linter: SetJavaScriptEnabled] Audit WebView.setJavaScriptEnabled call

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 41
Tracking Status
firefox41 --- fixed

People

(Reporter: mcomella, Assigned: mcomella)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

via lint:

 ../../src/main/java/org/mozilla/search/PostSearchFragment.java:59: Using setJavaScriptEnabled can introduce XSS vulnerabilities into you application, review carefully.

  56         webview.setWebViewClient(new ResultsWebViewClient());
  57 
  58         // This is required for our greasemonkey terror script.
  59         webview.getSettings().setJavaScriptEnabled(true);
  60 
  61         return mainView;

Priority: 6 / 10
Category: Security
Severity: Warning
Explanation: Using setJavaScriptEnabled
Your code should not invoke setJavaScriptEnabled if you are not sure that your app really requires JavaScript support.

More info: http://developer.android.com/guide/practices/security.html 

---

Margaret, my assumption is that this call is necessary and we should suppress the linter warning.

However, have you had this audited? Are we certain there are no vulnerabilities here?

I'll post a patch if so.
Flags: needinfo?(margaret.leibovic)
Bug 1173228 - Suppress SetJavaScriptEnabled linter warning. r?margaret

The function is required for a greasemonkey script and the code is expected to
be secure.
Attachment #8617707 - Flags: review?(margaret.leibovic)
Summary: Audit WebView.setJavaScriptEnabled call (SetJavaScriptEnabled linter warning) → [Linter: SetJavaScriptEnabled] Audit WebView.setJavaScriptEnabled call
Comment on attachment 8617707 [details]
MozReview Request: Bug 1173228 - Suppress SetJavaScriptEnabled linter warning. r?margaret

https://reviewboard.mozilla.org/r/10689/#review9667

Ship It!
Attachment #8617707 - Flags: review?(margaret.leibovic) → review+
(In reply to Michael Comella (:mcomella) from comment #0)
> via lint:
> 
>  ../../src/main/java/org/mozilla/search/PostSearchFragment.java:59: Using
> setJavaScriptEnabled can introduce XSS vulnerabilities into you application,
> review carefully.
> 
>   56         webview.setWebViewClient(new ResultsWebViewClient());
>   57 
>   58         // This is required for our greasemonkey terror script.
>   59         webview.getSettings().setJavaScriptEnabled(true);
>   60 
>   61         return mainView;
> 
> Priority: 6 / 10
> Category: Security
> Severity: Warning
> Explanation: Using setJavaScriptEnabled
> Your code should not invoke setJavaScriptEnabled if you are not sure that
> your app really requires JavaScript support.
> 
> More info: http://developer.android.com/guide/practices/security.html 
> 
> ---
> 
> Margaret, my assumption is that this call is necessary and we should
> suppress the linter warning.
> 
> However, have you had this audited? Are we certain there are no
> vulnerabilities here?
> 
> I'll post a patch if so.

For the record, we use this to load a style script to hide certain contents on the search results pages:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/search/java/org/mozilla/search/providers/SearchEngine.java#49

That "greasemonkey terror script" comment is a bit snarky, we should probably update it with something more useful :)
Flags: needinfo?(margaret.leibovic)
That injection seems very fragile – were any alternatives considered (e.g. talking to the providers directly to get a version of the page with those tags)?
Flags: needinfo?(margaret.leibovic)
https://hg.mozilla.org/mozilla-central/rev/12ab70b36685
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
(In reply to Michael Comella (:mcomella) from comment #4)
> That injection seems very fragile – were any alternatives considered (e.g.
> talking to the providers directly to get a version of the page with those
> tags)?

Yes, it's terrible. See bug 1059984.
Flags: needinfo?(margaret.leibovic)
You need to log in before you can comment on or make changes to this bug.