Opened on 03/13/2019 at 11:03:26 AM
Closed on 03/20/2019 at 05:06:10 PM
Last modified on 04/04/2019 at 09:43:40 AM
#7356 closed change (fixed)
Allow wrapping of function properties in abort-current-inline-script snippet
| Reported by: | agiammarchi | Assignee: | agiammarchi |
|---|---|---|---|
| Priority: | P2 | Milestone: | |
| Module: | Core | Keywords: | circumvention |
| Cc: | amr | Blocked By: | |
| Blocking: | Platform: | Unknown / Cross platform | |
| Ready: | yes | Confidential: | no |
| Tester: | Ross | Verified working: | yes |
| Review URL(s): |
https://gitlab.com/eyeo/adblockplus/adblockpluscore/merge_requests/28 |
||
Description (last modified by agiammarchi)
Background
We currently hook only into objects, making the ability to hook into commonly needed/used public static utilities, such as Object.keys or String.fromCharCode, impossible.
What to change
In lib/content/snippets.js, relax the split API check within the loop, to enable checks through global constructors too (typeof function).
Hints for testers
It should now be possible to prevent access to properties on functions. e.g. try the filter #$#abort-current-inline-script Object.keys and add <script>console.log(Object.keys({a: 1}))</script> to your HTML document; it should not print ["a"] in the console.
Attachments (0)
Change History (21)
comment:1 Changed on 03/13/2019 at 11:05:33 AM by agiammarchi
- Review URL(s) modified (diff)
- Verified working set
comment:2 Changed on 03/13/2019 at 01:11:00 PM by amrmak
- Cc amr added
comment:3 Changed on 03/13/2019 at 01:12:26 PM by amrmak
- Keywords circumvention added
comment:5 Changed on 03/13/2019 at 05:02:46 PM by mjethani
- Verified working unset
comment:7 in reply to: ↑ 6 ; follow-up: ↓ 8 Changed on 03/14/2019 at 09:02:07 AM by agiammarchi
Replying to mjethani:
The "Verified working" flag should not be set, that is what QA does.
fair enough, but in various people here have verified this actually works (I've created a build with this change in and shared it)
comment:8 in reply to: ↑ 7 Changed on 03/14/2019 at 09:56:03 AM by mjethani
Replying to agiammarchi:
Replying to mjethani:
The "Verified working" flag should not be set, that is what QA does.
fair enough, but in various people here have verified this actually works (I've created a build with this change in and shared it)
That's nice to know.
The flag though is for QA testing, which is a more formal process.
comment:9 Changed on 03/14/2019 at 10:00:53 AM by abpbot
A commit referencing this issue has landed:
Issue 7356 - Allow wrapping of function properties in snippets
comment:10 Changed on 03/20/2019 at 04:56:32 PM by abpbot
A commit referencing this issue has landed:
Issue 7356 - Allow wrapping of function properties in snippets
comment:11 Changed on 03/20/2019 at 04:59:00 PM by mjethani
- Description modified (diff)
- Owner set to agiammarchi
- Priority changed from Unknown to P2
- Ready set
- Summary changed from Relax abortCurrentInlineScript to Relax Allow wrapping of function properties in snippets
comment:12 Changed on 03/20/2019 at 04:59:20 PM by mjethani
- Summary changed from Relax Allow wrapping of function properties in snippets to Allow wrapping of function properties in snippets
comment:13 Changed on 03/20/2019 at 05:06:10 PM by mjethani
- Description modified (diff)
- Resolution set to fixed
- Status changed from new to closed
comment:14 Changed on 03/20/2019 at 05:39:11 PM by mjethani
- Description modified (diff)
comment:15 Changed on 03/28/2019 at 12:04:52 PM by Ross
This does not appear to be working in Chrome or Firefox.
With an external script:
if( window.blah ){
console.log("blah");
}
console.log("before");
console.log(Object.keys({a: 1}));
console.log("after");
On a test page:
<!DOCTYPE html>
<html>
<body>
<script src="script.js"></script>
</body>
</html>
The filter host#$#abort-on-property-read Object.keys does not appear to do anything. Have double checked that snippets are actually being applied to the page using the filter host#$#abort-on-property-read blah which works as expected. No errors appear in the page or extension background consoles. Have also tried using wrapping the console logs in a setTimeout.
ABP 3.5.0.2283
Chrome 73.0.3683.86 / Windows 10
Firefox 65.0.1 / Windows 10
comment:16 Changed on 03/28/2019 at 12:43:22 PM by agiammarchi
- Description modified (diff)
comment:17 Changed on 03/28/2019 at 12:44:18 PM by agiammarchi
my apologies the ticket had bad copy/paste mistake, it's abort-current-inline-script that should work.
the property read bit hasn't changed in here.
comment:18 Changed on 03/28/2019 at 01:43:56 PM by mjethani
There is currently no snippet called abort-on-function-call.
comment:19 Changed on 03/28/2019 at 01:49:30 PM by mjethani
- Summary changed from Allow wrapping of function properties in snippets to Allow wrapping of function properties in abort-current-inline-script snippet
comment:20 Changed on 03/28/2019 at 04:10:48 PM by Ross
- Tester changed from Unknown to Ross
- Verified working set
Done. Working as expected in the browsers below. It fails in Edge, but that looks to be because the snippet as a whole does not work in Edge (#7420). I could not get it working in Firefox Mobile either, but that might be related to #7410.
ABP 3.5.0.2283
Chrome 73.0.3683.86 / Windows 10
Chrome 49.0.2623.75 / Windows 10
Firefox 65.0.1 / Windows 10
Firefox 51.0 / Windows 10
Opera 58.0.3135.118 / Windows 10
Opera 36.0.2130.80 / Windows 10
comment:21 Changed on 04/04/2019 at 09:43:40 AM by mjethani
- Sensitive unset
The "Verified working" flag should not be set, that is what QA does.