Skip to content

Commit

Permalink
[WebUI] Add presubmit banrule to prevent chrome.send
Browse files Browse the repository at this point in the history
Prevent usage of chrome.send in ChromeOS WebUIs

Initially only preventing for ChromeOS WebUIs, we will look at expanding this to all of Chrome in the future.

Context:
https://chromium.googlesource.com/chromium/src/+/refs/heads/main/docs/security/handling-messages-from-web-content.md

Bug: 1382986
Change-Id: Ic12d43876613a6f2e514d9b1573cc5f75fe224c4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4011838
Reviewed-by: Daniel Cheng <[email protected]>
Commit-Queue: Clement Yan <[email protected]>
Reviewed-by: Giovanni Ortuno Urquidi <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1072640}
  • Loading branch information
Clement Yan authored and Chromium LUCI CQ committed Nov 17, 2022
1 parent 6ade24b commit 9b330cb
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 0 deletions.
32 changes: 32 additions & 0 deletions PRESUBMIT.py
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,32 @@ class BanRule:
),
)

_BANNED_JAVASCRIPT_FUNCTIONS : Sequence [BanRule] = (
BanRule(
r'/\bchrome\.send\b',
(
'The use of chrome.send is disallowed in Chrome (context: https://chromium.googlesource.com/chromium/src/+/refs/heads/main/docs/security/handling-messages-from-web-content.md).',
'Please use mojo instead for new webuis. https://docs.google.com/document/d/1RF-GSUoveYa37eoyZ9EhwMtaIwoW7Z88pIgNZ9YzQi4/edit#heading=h.gkk22wgk6wff',
),
True,
(
r'^(?!ash\/webui).+',
# TODO(crbug.com/1385601): pre-existing violations still need to be
# cleaned up.
'ash/webui/common/resources/multidevice_setup/multidevice_setup_browser_proxy.js',
'ash/webui/common/resources/quick_unlock/lock_screen_constants.js',
'ash/webui/common/resources/smb_shares/smb_browser_proxy.js',
'ash/webui/connectivity_diagnostics/resources/connectivity_diagnostics.js',
'ash/webui/diagnostics_ui/resources/diagnostics_browser_proxy.ts',
'ash/webui/multidevice_debug/resources/logs.js',
'ash/webui/multidevice_debug/resources/webui.js',
'ash/webui/projector_app/resources/annotator/trusted/annotator_browser_proxy.js',
'ash/webui/projector_app/resources/app/trusted/projector_browser_proxy.js',
'ash/webui/scanning/resources/scanning_browser_proxy.js',
),
),
)

_BANNED_OBJC_FUNCTIONS : Sequence[BanRule] = (
BanRule(
'addTrackingRect:',
Expand Down Expand Up @@ -2114,6 +2140,12 @@ def CheckForMatch(affected_file, line_num: int, line: str,
for ban_rule in _BANNED_JAVA_FUNCTIONS:
CheckForMatch(f, line_num, line, ban_rule)

file_filter = lambda f: f.LocalPath().endswith(('.js', '.ts'))
for f in input_api.AffectedFiles(file_filter=file_filter):
for line_num, line in f.ChangedContents():
for ban_rule in _BANNED_JAVASCRIPT_FUNCTIONS:
CheckForMatch(f, line_num, line, ban_rule)

file_filter = lambda f: f.LocalPath().endswith(('.mm', '.m', '.h'))
for f in input_api.AffectedFiles(file_filter=file_filter):
for line_num, line in f.ChangedContents():
Expand Down
14 changes: 14 additions & 0 deletions PRESUBMIT_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -2767,6 +2767,20 @@ def testChangeOwnerIsSecurityOwner(self):


class BannedTypeCheckTest(unittest.TestCase):
def testBannedJsFunctions(self):
input_api = MockInputApi()
input_api.files = [
MockFile('ash/webui/file.js',
['chrome.send(something);']),
MockFile('some/js/ok/file.js',
['chrome.send(something);']),
]

results = PRESUBMIT.CheckNoBannedFunctions(input_api, MockOutputApi())

self.assertEqual(1, len(results))
self.assertTrue('ash/webui/file.js' in results[0].message)
self.assertFalse('some/js/ok/file.js' in results[0].message)

def testBannedCppFunctions(self):
input_api = MockInputApi()
Expand Down

0 comments on commit 9b330cb

Please sign in to comment.