Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security] Shield feature "Allow scripts once" doesn't work #20503

Closed
palianskas opened this issue Jan 14, 2022 · 11 comments · Fixed by brave/brave-core#11928
Closed

[Security] Shield feature "Allow scripts once" doesn't work #20503

palianskas opened this issue Jan 14, 2022 · 11 comments · Fixed by brave/brave-core#11928

Comments

@palianskas
Copy link

Description

Allow scripts once or Allow on individual scripts don't work, all scripts are still blocked

Steps to Reproduce

  1. Turn on Scripts blocked option in Shield
  2. Click Allow scripts once or Allow on individual scripts

Actual result:

All scripts are still blocked

Before clicking Allow on www.w3schools.com scripts
image
After clicking Allow on www.w3schools.com scripts
image

Expected result:

Currently blocked scripts get loaded if Allow scripts once was clicked or individual scripts get loaded if Allow was clicked

Reproduces how often:

Easily reproduced

Brave version (brave://version info)

Brave | 1.34.80 Chromium: 97.0.4692.71 (Official Build) (64-bit)
Revision | adefa7837d02a07a604c1e6eff0b3a09422ab88d-refs/branch-heads/4692@{#1247}
OS | Windows 10 Version 21H1 (Build 19043.1415)

Version/Channel Information:

  • Can you reproduce this issue with the current release?
    Yes
  • Can you reproduce this issue with the beta channel?
    Yes
  • Can you reproduce this issue with the nightly channel?
    Not tested

Other Additional Information:

  • Does the issue resolve itself when disabling Brave Shields?
    Yes
  • Does the issue resolve itself when disabling Brave Rewards?
    No
  • Is the issue reproducible on the latest version of Chrome?
    Shield feature unavailable on Chrome

Miscellaneous Information:

No additional info

@ryanbr
Copy link

ryanbr commented Jan 17, 2022

Thanks for the report. Have chased down the regression in v1.35.17 Looking into it further.

Nightly v1.35.27 (BAD)
Nightly v1.35.20 (BAD)
Nightly v1.35.17 (BAD)
Nightly v1.35.16 (GOOD)
Nightly v1.35.15 (GOOD)
Nightly v1.35.8  (GOOD)

@mariospr
Copy link
Contributor

I'm afraid this is likely related to brave/brave-core@6d7031c where, besides adapting tests, we were forced to adapt UpdateContentSettingsToRendererFrames() in browser/brave_shields/brave_shields_web_contents_observer.cc to the removal of WebContents::GetAllFrames().

@iefremov @bridiver If any of you could take a look at this as Brave Shields experts, that would be great. It's possible that the way I adapted UpdateContentSettingsToRendererFrames() was not 100% correct in the end, but it's hard for me to know what exactly could be the problem here without deeper knowledge on how Shields work.

@kjozwiak kjozwiak added feature/shields/!scripts Blocking JavaScript with Shields QA/Yes regression labels Jan 18, 2022
@diracdeltas diracdeltas added the priority/P2 A bad problem. We might uplift this to the next planned release. label Jan 19, 2022
@diracdeltas
Copy link
Member

"Allow scripts once" is a pretty useful feature in shields - could we get it added to either manual QA runs or as an automated test to make sure this regression doesn't happen again?

@RangeAntelope
Copy link

Greetings,
I'm using Brave Browser Version 1.34.80 Chromium: 97.0.4692.71 (Official Build) (64-bit) - Windows 10 Home.
Over the past few days when I try to use ALLOW SCRIPTS ONCE (and say there's 8 scripts wanting to execute) it just flips the screen and I have 8 scripts waiting to execute again (it does NOT allow them to execute one time as in the past). When I've needed to access a site I don't want to give permanent JAVA Script executions to, I have to click the ALLOW button and then NoScript stops them all from flooding in and have execution permissions.
This is a hassle... ALLOW SCRIPTS ONCE used to work JUST ONCE and I didn't have to depend on NoScript as much.
I'm just wondering what might be going on?

@goodov goodov added this to the 1.35.x - Beta milestone Jan 21, 2022
@LaurenWags LaurenWags changed the title Shield feature "Allow scripts once" doesn't work [Security] Shield feature "Allow scripts once" doesn't work Jan 24, 2022
@LaurenWags
Copy link
Member

Added QA/Blocked label until this fix is uplifted to 1.35.x.

@goodov
Copy link
Member

goodov commented Jan 27, 2022

Allow scripts once doesn't work even in release currently (it works partially(?)). Reopening.

@goodov goodov reopened this Jan 27, 2022
@goodov
Copy link
Member

goodov commented Jan 27, 2022

Apparently we don't track blocked embedded scripts. Seems like this was always an issue, and if a website contains such script in the page body, then we still don't allow it when "Allow scripts once" is clicked. Also iframes sometimes don't work with "Allow scripts once" action.

@kjozwiak
Copy link
Member

Apparently we don't track blocked embedded scripts. Seems like this was always an issue, and if a website contains such script in the page body, then we still don't allow it when "Allow scripts once" is clicked. Also iframes don't work with "Allow scripts once" action.

Closing this issue as brave/brave-core#11928 is already associated with this issue. When QAing the above, I ran into #20744. This will be fixed via brave/brave-core#12023 and will most likely be uplifted along side this one.

@kjozwiak
Copy link
Member

Removing QA/Blocked as this was uplifted into 1.35.x. Should be checked with 1.35.97 or higher.

@stephendonner
Copy link

stephendonner commented Jan 28, 2022

Verified PASSED using

Brave 1.35.97 Chromium: 98.0.4758.72 (Official Build) (64-bit)
Revision d63e30f73734c5326037b0d94eceed77168d95e1-refs/branch-heads/4758@{#905}
OS Linux

Steps:

  1. new profile
  2. launched Brave
  3. via brave://settings/shields, enabled Block scripts
  4. loaded w3schools.com
  5. clicked on the Shields icon in the URL bar
  6. noted there were 3 scripts blocked
  7. clicked on Allow scripts once
  8. confirmed script was unblocked
  9. clicked again
  10. confirmed last script was unblocked
scripts not blocked Block scripts scripts blocked unblocked using Allow scripts once unblocked using Allow scripts once
Screen Shot 2022-01-28 at 9 36 30 AM Screen Shot 2022-01-28 at 9 29 49 AM Screen Shot 2022-01-28 at 9 30 49 AM Screen Shot 2022-01-28 at 9 30 57 AM Screen Shot 2022-01-28 at 9 31 06 AM

Verified PASSED using

Brave 1.35.97 Chromium: 98.0.4758.72 (Official Build) (x86_64)
Revision d63e30f73734c5326037b0d94eceed77168d95e1-refs/branch-heads/4758@{#905}
OS macOS Version 11.6.1 (Build 20G224)

Steps:

  1. new profile
  2. launched Brave
  3. via brave://settings/shields, enabled Block scripts
  4. loaded w3schools.com
  5. clicked on the Shields icon in the URL bar
  6. noted there were 3 scripts blocked
  7. clicked on Allow scripts once
  8. confirmed script was unblocked
  9. clicked again
  10. confirmed last script was unblocked
scripts not blocked Block scripts scripts blocked unblocked using Allow scripts once unblocked using Allow scripts once
Screen Shot 2022-01-28 at 1 38 52 PM Screen Shot 2022-01-28 at 1 36 29 PM Screen Shot 2022-01-28 at 9 30 49 AM Screen Shot 2022-01-28 at 9 30 57 AM Screen Shot 2022-01-28 at 9 31 06 AM

Verification PASSED on

Brave | 1.35.98 Chromium: 98.0.4758.74 (Official Build) (64-bit)
-- | --
Revision | d0fe1ec4df090cd3eb02b591228505e12ea476e9-refs/branch-heads/4758@{#935}
OS | Windows 10 Version 21H2 (Build 19044.1466)

  1. new profile
  2. launched Brave
  3. via brave://settings/shields, enabled Block scripts
  4. loaded w3schools.com
  5. clicked on the Shields icon in the URL bar
  6. noted there were 4 scripts blocked
  7. clicked on Allow scripts once
  8. confirmed script was unblocked
  9. clicked again
  10. confirmed last script was unblocked
scripts not blocked Block scripts scripts blocked unblocked using Allow scripts once unblocked using Allow scripts once
image image image image image

@RangeAntelope
Copy link

RangeAntelope commented Jan 28, 2022

That looks very promising! I don't know if MAC OS using Brave will/would be effective using Windows 10 + Brave Version 1.34.81 Chromium: 97.0.4692.99 (Official Build) (64-bit) which is the latest version according to ABOUT BRAVE.

I'm assuming that the ALLOW SCRIPTS ONCE function will be fixed in an upcoming 1.35.+ version so we'll see.
Thank you so much for attending to this so rapidly!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants