Skip to content
This repository has been archived by the owner on Feb 1, 2019. It is now read-only.

Commit

Permalink
Disallow ctypes
Browse files Browse the repository at this point in the history
  • Loading branch information
wagnerand committed Mar 30, 2017
1 parent 633098a commit ea8566e
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 47 deletions.
14 changes: 9 additions & 5 deletions tests/test_js_signing.py
Expand Up @@ -335,15 +335,19 @@ def test(prop):
yield test, prop

def test_ctypes(self):
"""Tests that usage of `ctypes` generates warnings."""
"""Tests that usage of `ctypes` generates errors."""

self.run_script("""
ctypes.open("libc.so.6");
""")
self.assert_failed(with_warnings=[
{'id': ('js', 'traverser', 'dangerous_global'),
'editors_only': True,
'signing_severity': 'high'}])
self.assert_failed(with_errors=[
{'id': ('js', 'traverser', 'forbidden_global')}])

self.run_script("""
Components.utils.import("resource://gre/modules/ctypes.jsm")
""")
self.assert_failed(with_errors=[
{'id': ('js', 'traverser', 'forbidden_global')}])

def test_nsIProcess(self):
"""Tests that usage of `nsIProcess` generates warnings."""
Expand Down
67 changes: 45 additions & 22 deletions validator/testcases/javascript/actions.py
Expand Up @@ -439,28 +439,51 @@ def _call_expression(traverser, node):
column=traverser.position,
context=traverser.context)

if member.is_global and callable(member.value.get('dangerous', None)):
result = member.value['dangerous'](a=args, t=traverser._traverse_node,
e=traverser.err)
name = member.value.get('name', '')

if result and name:
kwargs = {
'err_id': ('testcases_javascript_actions', '_call_expression',
'called_dangerous_global'),
'warning': '`%s` called in potentially dangerous manner' %
member.value['name'],
'description':
'The global `%s` function was called using a set '
'of dangerous parameters. Calls of this nature '
'are deprecated.' % member.value['name']}

if isinstance(result, DESCRIPTION_TYPES):
kwargs['description'] = result
elif isinstance(result, dict):
kwargs.update(result)

traverser.warning(**kwargs)
if member.is_global:
if callable(member.value.get('dangerous', None)):
result = member.value['dangerous'](a=args, t=traverser._traverse_node,
e=traverser.err)
name = member.value.get('name', '')

if result and name:
kwargs = {
'err_id': ('testcases_javascript_actions', '_call_expression',
'called_dangerous_global'),
'warning': '`%s` called in potentially dangerous manner' %
member.value['name'],
'description':
'The global `%s` function was called using a set '
'of dangerous parameters. Calls of this nature '
'are deprecated.' % member.value['name']}

if isinstance(result, DESCRIPTION_TYPES):
kwargs['description'] = result
elif isinstance(result, dict):
kwargs.update(result)

traverser.warning(**kwargs)

if callable(member.value.get('forbidden', None)):
result = member.value['forbidden'](a=args, t=traverser._traverse_node,
e=traverser.err)
name = member.value.get('name', '')

if result and name:
kwargs = {
'err_id': ('testcases_javascript_actions', '_call_expression',
'called_forbidden_global'),
'warning': ' Forbidden `%s` called' %
member.value['name'],
'description':
'The forbidden global `%s` function was called.'
% member.value['name']}

if isinstance(result, DESCRIPTION_TYPES):
kwargs['description'] = result
elif isinstance(result, dict):
kwargs.update(result)

traverser.error(**kwargs)

elif (node['callee']['type'] == 'MemberExpression' and
node['callee']['property']['type'] == 'Identifier'):
Expand Down
6 changes: 5 additions & 1 deletion validator/testcases/javascript/jstypes.py
Expand Up @@ -256,7 +256,11 @@ def apply_value(name):
if name in self.value:
output.value[name] = self.value[name]

map(apply_value, ('dangerous', 'readonly', 'context', 'name'))
map(apply_value, ('forbidden',
'dangerous',
'readonly',
'context',
'name'))
output.is_global = True
output.context = self.context
return output
Expand Down
25 changes: 6 additions & 19 deletions validator/testcases/javascript/predefinedentities.py
Expand Up @@ -717,25 +717,12 @@ def interface_obj(iface):
u'getInstallForURL': ADDON_INSTALL_METHOD,
u'installAddonsFromWebpage': ADDON_INSTALL_METHOD}},

u'ctypes': {'dangerous': {
u'ctypes': {'forbidden': {
'description': (
'Insufficiently meticulous use of ctypes can lead to serious, '
'and often exploitable, errors. The use of bundled binary code, '
'or access to system libraries, may allow for add-ons to '
'perform unsafe operations. All ctypes use must be carefully '
'reviewed by a qualified reviewer.'),
'editors_only': True,
'signing_help': ('Please try to avoid interacting with or bundling '
'native binaries whenever possible. If you are '
'bundling binaries for performance reasons, please '
'consider alternatives such as Emscripten '
'(http://mzl.la/1KrSUh2), JavaScript typed arrays '
'(http://mzl.la/1Iw02sr), and Worker threads '
'(http://mzl.la/1OGfAcc).',
'Any code which makes use of the `ctypes` API '
'must undergo manual code review for at least one '
'submission.'),
'signing_severity': 'high'}},
'Starting with Firefox 53, add-ons loading binary libraries are '
'not allowed. Native Messaging is the only allowed method to '
'communicate with external binaries. For more information, please '
'refer to https://developer.mozilla.org/en-US/Add-ons/WebExtensions/Native_messaging')}},

u'document':
{'value':
Expand Down Expand Up @@ -925,7 +912,7 @@ def interface_obj(iface):
'Care should be taken to ensure that '
'this is done safely.')}},
u'import':
{'dangerous':
{'forbidden':
lambda a, t, e:
a and 'ctypes.jsm' in _get_as_str(t(a[0]))},

Expand Down
21 changes: 21 additions & 0 deletions validator/testcases/javascript/traverser.py
Expand Up @@ -325,6 +325,27 @@ def _build_global(self, name, entity):
self._debug('DANGEROUS')
self.warning(**kwargs)

if not callable(entity.get('forbidden')):
dang = entity.get('forbidden')

if callable(dang):
dang = dang(self._traverse_node, self.err)

if dang:
kwargs = dict(
err_id=('js', 'traverser', 'forbidden_global'),
error='Access to the `%s` global' % name,
description='Access to the `%s` property is '
'forbidden.' % name)

if isinstance(dang, DESCRIPTION_TYPES):
kwargs['description'] = dang
elif isinstance(dang, dict):
kwargs.update(dang)

self._debug('FORBIDDEN')
self.error(**kwargs)

entity.setdefault('name', name)

# Build out the wrapper object from the global definition.
Expand Down

0 comments on commit ea8566e

Please sign in to comment.