Skip to content

Commit

Permalink
There is a need for TaintedString and TaintedBytes
Browse files Browse the repository at this point in the history
  • Loading branch information
gotcha committed Jan 28, 2018
1 parent 104def6 commit 8d82a7d
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 31 deletions.
2 changes: 1 addition & 1 deletion buildout.cfg
Expand Up @@ -19,7 +19,7 @@ parts =
checkversions
requirements
sources-dir = develop
auto-checkout =
auto-checkout = AccessControl


[testenv]
Expand Down
2 changes: 1 addition & 1 deletion sources.cfg
Expand Up @@ -4,7 +4,7 @@ github_push = git@github.com:zopefoundation

[sources]
# Zope-specific
AccessControl = git ${remotes:github}/AccessControl pushurl=${remotes:github_push}/AccessControl branch=master
AccessControl = git ${remotes:github}/AccessControl pushurl=${remotes:github_push}/AccessControl branch=TaintedBytes
Acquisition = git ${remotes:github}/Acquisition pushurl=${remotes:github_push}/Acquisition
AuthEncoding = git ${remotes:github}/AuthEncoding pushurl=${remotes:github_push}/AuthEncoding
DateTime = git ${remotes:github}/DateTime pushurl=${remotes:github_push}/DateTime
Expand Down
53 changes: 27 additions & 26 deletions src/ZPublisher/HTTPRequest.py
Expand Up @@ -29,7 +29,8 @@
)
import time

from AccessControl.tainted import TaintedString
from AccessControl.tainted import should_be_tainted
from AccessControl.tainted import taint_string
import pkg_resources
from six import binary_type
from six import PY2
Expand Down Expand Up @@ -462,11 +463,11 @@ def __init__(self, stdin, environ, response, clean=0):
parse_cookie(k, cookies)
for k, v in cookies.items():
istainted = 0
if '<' in k:
k = TaintedString(k)
if should_be_tainted(k):
k = taint_string(k)
istainted = 1
if '<' in v:
v = TaintedString(v)
if should_be_tainted(v):
v = taint_string(v)
istainted = 1
if istainted:
taintedcookies[k] = v
Expand Down Expand Up @@ -632,8 +633,8 @@ def processInputs(

# If the key is tainted, mark it so as well.
tainted_key = key
if '<' in key:
tainted_key = TaintedString(key)
if should_be_tainted(key):
tainted_key = taint_string(key)

if flags:

Expand All @@ -648,11 +649,11 @@ def processInputs(

# Update the tainted_key if necessary
tainted_key = key
if '<' in key:
tainted_key = TaintedString(key)
if should_be_tainted(key):
tainted_key = taint_string(key)

# Attributes cannot hold a <.
if '<' in attr:
if should_be_tainted(attr):
raise ValueError(
"%s is not a valid record attribute name" %
escape(attr, True))
Expand All @@ -678,16 +679,16 @@ def processInputs(
# Flag potentially unsafe values
if converter_type in ('string', 'required', 'text',
'ustring', 'utext'):
if not isFileUpload and '<' in item:
tainted = TaintedString(item)
if not isFileUpload and should_be_tainted(item):
tainted = taint_string(item)
elif converter_type in ('tokens', 'lines',
'utokens', 'ulines'):
is_tainted = 0
tainted = item[:]
for i in range(len(tainted)):
if '<' in tainted[i]:
if should_be_tainted(tainted[i]):
is_tainted = 1
tainted[i] = TaintedString(tainted[i])
tainted[i] = taint_string(tainted[i])
if not is_tainted:
tainted = None

Expand All @@ -708,13 +709,13 @@ def processInputs(
else:
raise

elif not isFileUpload and '<' in item:
elif not isFileUpload and should_be_tainted(item):
# Flag potentially unsafe values
tainted = TaintedString(item)
tainted = taint_string(item)

# If the key is tainted, we need to store stuff in the
# tainted dict as well, even if the value is safe.
if '<' in tainted_key and tainted is None:
if should_be_tainted(tainted_key) and tainted is None:
tainted = item

# Determine which dictionary to use
Expand Down Expand Up @@ -916,9 +917,9 @@ def processInputs(
# This branch is for case when no type was specified.
mapping_object = form

if not isFileUpload and '<' in item:
tainted = TaintedString(item)
elif '<' in key:
if not isFileUpload and should_be_tainted(item):
tainted = taint_string(item)
elif should_be_tainted(key):
tainted = item

# Insert in dictionary
Expand Down Expand Up @@ -964,8 +965,8 @@ def processInputs(
if defaults:
for key, value in defaults.items():
tainted_key = key
if '<' in key:
tainted_key = TaintedString(key)
if should_be_tainted(key):
tainted_key = taint_string(key)

if key not in form:
# if the form does not have the key,
Expand Down Expand Up @@ -1116,8 +1117,8 @@ def processInputs(
if k in form:
# If the form has the split key get its value
tainted_split_key = k
if '<' in k:
tainted_split_key = TaintedString(k)
if should_be_tainted(k):
tainted_split_key = taint_string(k)
item = form[k]
if isinstance(item, record):
# if the value is mapped to a record, check if it
Expand Down Expand Up @@ -1151,8 +1152,8 @@ def processInputs(
else:
# the form does not have the split key
tainted_key = key
if '<' in key:
tainted_key = TaintedString(key)
if should_be_tainted(key):
tainted_key = taint_string(key)
if key in form:
# if it has the original key, get the item
# convert it to a tuple
Expand Down
7 changes: 4 additions & 3 deletions src/ZPublisher/tests/testHTTPRequest.py
Expand Up @@ -16,6 +16,7 @@
import unittest

from six import PY2
from AccessControl.tainted import should_be_tainted
from zExceptions import NotFound
from zope.component import provideAdapter
from zope.i18n.interfaces import IUserPreferredLanguages
Expand Down Expand Up @@ -161,8 +162,8 @@ def _valueIsOrHoldsTainted(self, val):
retval = 0

if isinstance(val, TaintedString):
self.assertFalse(
'<' not in val,
self.assertTrue(
should_be_tainted(val),
"%r is not dangerous, no taint required." % val)
retval = 1

Expand Down Expand Up @@ -289,7 +290,7 @@ def test_processInputs_w_simple_marshalling(self):
self.assertEqual(req['bign'], 45)
self.assertEqual(req['fract'], 4.2)
self.assertEqual(req['morewords'], 'one\ntwo\n')
self.assertEqual(req['multiline'], ['one', 'two'])
self.assertEqual(req['multiline'], [b'one', b'two'])
self.assertEqual(req['num'], 42)
self.assertEqual(req['words'], 'Some words')

Expand Down

0 comments on commit 8d82a7d

Please sign in to comment.