From 1de5375962306b1ce04c553eb38ad5237839959f Mon Sep 17 00:00:00 2001 From: kansi Date: Tue, 31 Oct 2017 15:16:59 +0530 Subject: [PATCH 1/3] Validate asset data keys --- bigchaindb/common/utils.py | 19 ++++++++++++++++++- bigchaindb/models.py | 4 +++- tests/web/test_transactions.py | 28 ++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/bigchaindb/common/utils.py b/bigchaindb/common/utils.py index f6f671db..fd9386e1 100644 --- a/bigchaindb/common/utils.py +++ b/bigchaindb/common/utils.py @@ -1,7 +1,10 @@ import time - +import re import rapidjson +import bigchaindb +from bigchaindb.common.exceptions import ValidationError + def gen_timestamp(): """The Unix time, rounded to the nearest second. @@ -46,3 +49,17 @@ def deserialize(data): string. """ return rapidjson.loads(data) + + +def validate_asset_data_keys(tx_body): + backend = bigchaindb.config['database']['backend'] + + if backend == 'mongodb': + data = tx_body['asset'].get('data', {}) + keys = data.keys() if data else [] + for key in keys: + if re.search(r'^[$]|\.', key): + error_str = ('Invalid key name "{}" in asset object. The ' + 'key name cannot contain characters ' + '"." and "$"').format(key) + raise ValidationError(error_str) from ValueError() diff --git a/bigchaindb/models.py b/bigchaindb/models.py index c8ad9dd3..9704372d 100644 --- a/bigchaindb/models.py +++ b/bigchaindb/models.py @@ -8,7 +8,8 @@ from bigchaindb.common.exceptions import (InvalidHash, InvalidSignature, SybilError, DuplicateTransaction) from bigchaindb.common.transaction import Transaction -from bigchaindb.common.utils import gen_timestamp, serialize +from bigchaindb.common.utils import (gen_timestamp, serialize, + validate_asset_data_keys) from bigchaindb.common.schema import validate_transaction_schema @@ -84,6 +85,7 @@ class Transaction(Transaction): @classmethod def from_dict(cls, tx_body): validate_transaction_schema(tx_body) + validate_asset_data_keys(tx_body) return super().from_dict(tx_body) @classmethod diff --git a/tests/web/test_transactions.py b/tests/web/test_transactions.py index acea8c2c..4c2a6ed9 100644 --- a/tests/web/test_transactions.py +++ b/tests/web/test_transactions.py @@ -47,6 +47,34 @@ def test_post_create_transaction_endpoint(b, client): assert res.json['outputs'][0]['public_keys'][0] == user_pub +@pytest.mark.parametrize("key,expected_status_code", [ + ('bad.key', 400), + ('$bad.key', 400), + ('$badkey', 400), + ('good_key', 202) +]) +@pytest.mark.assetkey +@pytest.mark.bdb +def test_post_create_transaction_with_invalid_asset_key(b, client, key, expected_status_code): + from bigchaindb.models import Transaction + from bigchaindb.backend.mongodb.connection import MongoDBConnection + user_priv, user_pub = crypto.generate_key_pair() + + if isinstance(b.connection, MongoDBConnection): + tx = Transaction.create([user_pub], [([user_pub], 1)], + asset={key: 'random_value'}) + tx = tx.sign([user_priv]) + res = client.post(TX_ENDPOINT, data=json.dumps(tx.to_dict())) + + assert res.status_code == expected_status_code + if res.status_code == 400: + expected_error_message = ( + 'Invalid transaction (ValidationError): Invalid key name "{}" ' + 'in asset object. The key name cannot contain characters ' + '"." and "$"').format(key) + assert res.json['message'] == expected_error_message + + @patch('bigchaindb.web.views.base.logger') def test_post_create_transaction_with_invalid_id(mock_logger, b, client): from bigchaindb.common.exceptions import InvalidHash From f3da30aea082347a8ae72d1b8ddd97e0def5d37d Mon Sep 17 00:00:00 2001 From: kansi Date: Wed, 1 Nov 2017 17:26:16 +0530 Subject: [PATCH 2/3] Validate nested keys for asset.data and metadata --- bigchaindb/common/utils.py | 28 +++++++++++++++++++--------- bigchaindb/models.py | 5 +++-- tests/web/test_transactions.py | 29 ++++++++++++++++++----------- 3 files changed, 40 insertions(+), 22 deletions(-) diff --git a/bigchaindb/common/utils.py b/bigchaindb/common/utils.py index fd9386e1..35163f14 100644 --- a/bigchaindb/common/utils.py +++ b/bigchaindb/common/utils.py @@ -51,15 +51,25 @@ def deserialize(data): return rapidjson.loads(data) -def validate_asset_data_keys(tx_body): +def validate_txn_obj(obj_name, obj, key, validation_fun): backend = bigchaindb.config['database']['backend'] if backend == 'mongodb': - data = tx_body['asset'].get('data', {}) - keys = data.keys() if data else [] - for key in keys: - if re.search(r'^[$]|\.', key): - error_str = ('Invalid key name "{}" in asset object. The ' - 'key name cannot contain characters ' - '"." and "$"').format(key) - raise ValidationError(error_str) from ValueError() + data = obj.get(key, {}) or {} + validate_all_keys(obj_name, data, validation_fun) + + +def validate_all_keys(obj_name, obj, validation_fun): + for key, value in obj.items(): + validation_fun(obj_name, key) + if type(value) is dict: + validate_all_keys(obj_name, value, validation_fun) + return + + +def validate_key(obj_name, key): + if re.search(r'^[$]|\.|\x00', key): + error_str = ('Invalid key name "{}" in {} object. The ' + 'key name cannot contain characters ' + '".", "$" or null characters').format(key, obj_name) + raise ValidationError(error_str) from ValueError() diff --git a/bigchaindb/models.py b/bigchaindb/models.py index 9704372d..1ecd964e 100644 --- a/bigchaindb/models.py +++ b/bigchaindb/models.py @@ -9,7 +9,7 @@ from bigchaindb.common.exceptions import (InvalidHash, InvalidSignature, DuplicateTransaction) from bigchaindb.common.transaction import Transaction from bigchaindb.common.utils import (gen_timestamp, serialize, - validate_asset_data_keys) + validate_txn_obj, validate_key) from bigchaindb.common.schema import validate_transaction_schema @@ -85,7 +85,8 @@ class Transaction(Transaction): @classmethod def from_dict(cls, tx_body): validate_transaction_schema(tx_body) - validate_asset_data_keys(tx_body) + validate_txn_obj('asset', tx_body['asset'], 'data', validate_key) + validate_txn_obj('metadata', tx_body, 'metadata', validate_key) return super().from_dict(tx_body) @classmethod diff --git a/tests/web/test_transactions.py b/tests/web/test_transactions.py index 4c2a6ed9..e5034697 100644 --- a/tests/web/test_transactions.py +++ b/tests/web/test_transactions.py @@ -47,22 +47,29 @@ def test_post_create_transaction_endpoint(b, client): assert res.json['outputs'][0]['public_keys'][0] == user_pub -@pytest.mark.parametrize("key,expected_status_code", [ - ('bad.key', 400), - ('$bad.key', 400), - ('$badkey', 400), - ('good_key', 202) +@pytest.mark.parametrize("field", ['asset', 'metadata']) +@pytest.mark.parametrize("value,err_key,expected_status_code", [ + ({'bad.key': 'v'}, 'bad.key', 400), + ({'$bad.key': 'v'}, '$bad.key', 400), + ({'$badkey': 'v'}, '$badkey', 400), + ({'bad\x00key': 'v'}, 'bad\x00key', 400), + ({'good_key': {'bad.key': 'v'}}, 'bad.key', 400), + ({'good_key': 'v'}, 'good_key', 202) ]) -@pytest.mark.assetkey @pytest.mark.bdb -def test_post_create_transaction_with_invalid_asset_key(b, client, key, expected_status_code): +def test_post_create_transaction_with_invalid_key(b, client, field, value, + err_key, expected_status_code): from bigchaindb.models import Transaction from bigchaindb.backend.mongodb.connection import MongoDBConnection user_priv, user_pub = crypto.generate_key_pair() if isinstance(b.connection, MongoDBConnection): - tx = Transaction.create([user_pub], [([user_pub], 1)], - asset={key: 'random_value'}) + if field == 'asset': + tx = Transaction.create([user_pub], [([user_pub], 1)], + asset=value) + elif field == 'metadata': + tx = Transaction.create([user_pub], [([user_pub], 1)], + metadata=value) tx = tx.sign([user_priv]) res = client.post(TX_ENDPOINT, data=json.dumps(tx.to_dict())) @@ -70,8 +77,8 @@ def test_post_create_transaction_with_invalid_asset_key(b, client, key, expected if res.status_code == 400: expected_error_message = ( 'Invalid transaction (ValidationError): Invalid key name "{}" ' - 'in asset object. The key name cannot contain characters ' - '"." and "$"').format(key) + 'in {} object. The key name cannot contain characters ' + '".", "$" or null characters').format(err_key, field) assert res.json['message'] == expected_error_message From 8fdf8f6ca6cfdc3c20ac863c1b205dcfe365894e Mon Sep 17 00:00:00 2001 From: kansi Date: Thu, 2 Nov 2017 18:02:11 +0530 Subject: [PATCH 3/3] Added docstrings --- bigchaindb/common/utils.py | 43 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/bigchaindb/common/utils.py b/bigchaindb/common/utils.py index 35163f14..e472f380 100644 --- a/bigchaindb/common/utils.py +++ b/bigchaindb/common/utils.py @@ -52,6 +52,22 @@ def deserialize(data): def validate_txn_obj(obj_name, obj, key, validation_fun): + """Validates value associated to `key` in `obj` by applying + `validation_fun`. + + Args: + obj_name (str): name for `obj` being validated. + obj (dict): dictonary object. + key (str): key to be validated in `obj`. + validation_fun (function): function used to validate the value + of `key`. + + Returns: + None: indicates validation successfull + + Raises: + ValidationError: `validation_fun` will raise this error on failure + """ backend = bigchaindb.config['database']['backend'] if backend == 'mongodb': @@ -60,6 +76,20 @@ def validate_txn_obj(obj_name, obj, key, validation_fun): def validate_all_keys(obj_name, obj, validation_fun): + """Validates all (nested) keys in `obj` by using `validation_fun` + + Args: + obj_name (str): name for `obj` being validated. + obj (dict): dictonary object. + validation_fun (function): function used to validate the value + of `key`. + + Returns: + None: indicates validation successfull + + Raises: + ValidationError: `validation_fun` will raise this error on failure + """ for key, value in obj.items(): validation_fun(obj_name, key) if type(value) is dict: @@ -68,6 +98,19 @@ def validate_all_keys(obj_name, obj, validation_fun): def validate_key(obj_name, key): + """Check if `key` contains ".", "$" or null characters + https://docs.mongodb.com/manual/reference/limits/#Restrictions-on-Field-Names + + Args: + obj_name (str): object name to use when raising exception + key (str): key to validated + + Returns: + None: indicates validation successfull + + Raises: + ValidationError: raise execption incase of regex match. + """ if re.search(r'^[$]|\.|\x00', key): error_str = ('Invalid key name "{}" in {} object. The ' 'key name cannot contain characters '