Skip to content

Commit

Permalink
fix(dpp)!: data contract validation issues (#1851)
Browse files Browse the repository at this point in the history
  • Loading branch information
@shumkov
shumkov authored Jul 9, 2024
1 parent 37520df commit bff4b65
Show file tree
Hide file tree
Showing 25 changed files with 287 additions and 562 deletions.
9 changes: 5 additions & 4 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/rs-dpp/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ getrandom = { version = "0.2", features = ["js"] }
hex = { version = "0.4" }
integer-encoding = { version = "4.0.0" }
itertools = { version = "0.12.1" }
jsonschema = { git = "https://github.com/fominok/jsonschema-rs", branch = "feat-unevaluated-properties", default-features = false, features = [
jsonschema = { git = "https://github.com/dashpay/jsonschema-rs", branch = "configure_regexp", default-features = false, features = [
"draft202012",
], optional = true }
lazy_static = { version = "1.4" }
Expand Down
124 changes: 1 addition & 123 deletions packages/rs-dpp/schema/meta_schemas/document/v0/document-meta.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,6 @@
"type": "boolean",
"const": true
},
"prefixItems": {
"$ref": "#/$defs/documentSchemaArray"
},
"items": true,
"position": {
"type": "integer",
"minimum": 0
Expand All @@ -153,16 +149,6 @@
"type": "string",
"const": "array"
}
},
"not": {
"properties": {
"items": {
"type": "array"
}
},
"required": [
"items"
]
}
},
"contentMediaType": {
Expand Down Expand Up @@ -192,52 +178,6 @@
]
}
},
"uniqueItems": {
"description": "prevent slow validation of large non-scalar arrays",
"if": {
"properties": {
"uniqueItems": {
"const": true
},
"items": {
"type": "object",
"properties": {
"type": {
"anyOf": [
{
"type": "string",
"enum": [
"object",
"array"
]
},
{
"type": "array",
"contains": {
"enum": [
"object",
"array"
]
}
}
]
}
}
}
}
},
"then": {
"properties": {
"maxItems": {
"type": "number",
"maximum": 100000
}
},
"required": [
"maxItems"
]
}
},
"pattern": {
"description": "prevent slow pattern matching of large strings",
"properties": {
Expand All @@ -263,18 +203,6 @@
"required": [
"maxLength"
]
},
"prefixItems": {
"$comment": "array must not contain undefined item sub schemas",
"properties": {
"items": {
"type": "boolean",
"const": false
}
},
"required": [
"items"
]
}
},
"allOf": [
Expand Down Expand Up @@ -326,55 +254,6 @@
]
}
},
{
"$comment": "array must contain items",
"if": {
"properties": {
"type": {
"const": "array"
}
},
"required": [
"type"
],
"not": {
"properties": {
"byteArray": true
},
"required": [
"byteArray"
]
}
},
"then": {
"properties": {
"items": true
},
"required": [
"items"
]
}
},
{
"$comment": "array without prefixItems must contain items sub schema",
"if": {
"not": {
"properties": {
"prefixItems": true
},
"required": [
"prefixItems"
]
}
},
"then": {
"properties": {
"items": {
"$ref": "#/$defs/documentSchema"
}
}
}
},
{
"$comment": "all object properties must be defined",
"if": {
Expand Down Expand Up @@ -467,12 +346,11 @@
"signatureSecurityLevelRequirement": {
"type": "integer",
"enum": [
0,
1,
2,
3
],
"description": "Public key security level. 0 - Master, 1 - Critical, 2 - High, 3 - Medium. If none specified, High level is used"
"description": "Public key security level. 1 - Critical, 2 - High, 3 - Medium. If none specified, High level is used"
},
"documentsKeepHistory": {
"type": "boolean",
Expand Down
38 changes: 0 additions & 38 deletions ...a_contract/document_type/class_methods/create_document_types_from_document_schemas/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,41 +70,3 @@ impl DocumentType {
}
}
}

#[cfg(test)]
mod tests {
use super::*;

use crate::consensus::basic::data_contract::DocumentTypesAreMissingError;
use crate::consensus::basic::BasicError;
use crate::consensus::ConsensusError;
use crate::data_contract::errors::DataContractError;
use assert_matches::assert_matches;
use platform_value::Identifier;
use std::ops::Deref;

#[test]
pub fn should_not_allow_creating_document_types_with_empty_schema() {
let id = Identifier::random();

let result = DocumentType::create_document_types_from_document_schemas(
id,
Default::default(),
None,
false,
false,
false,
false,
&mut vec![],
PlatformVersion::latest(),
);

assert_matches!(result, Err(ProtocolError::ConsensusError(e)) => {
assert_matches!(e.deref(), ConsensusError::BasicError(BasicError::ContractError(
DataContractError::DocumentTypesAreMissingError(
DocumentTypesAreMissingError { .. }
)
)));
});
}
}
38 changes: 38 additions & 0 deletions ...ontract/document_type/class_methods/create_document_types_from_document_schemas/v0/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,41 @@ impl DocumentTypeV0 {
Ok(contract_document_types)
}
}

#[cfg(test)]
mod tests {
use super::*;

use crate::consensus::basic::data_contract::DocumentTypesAreMissingError;
use crate::consensus::basic::BasicError;
use crate::consensus::ConsensusError;
use crate::data_contract::errors::DataContractError;
use assert_matches::assert_matches;
use platform_value::Identifier;
use std::ops::Deref;

#[test]
pub fn should_not_allow_creating_document_types_with_empty_schema() {
let id = Identifier::random();

let result = DocumentType::create_document_types_from_document_schemas(
id,
Default::default(),
None,
false,
false,
false,
false,
&mut vec![],
PlatformVersion::latest(),
);

assert_matches!(result, Err(ProtocolError::ConsensusError(e)) => {
assert_matches!(e.deref(), ConsensusError::BasicError(BasicError::ContractError(
DataContractError::DocumentTypesAreMissingError(
DocumentTypesAreMissingError { .. }
)
)));
});
}
}
56 changes: 9 additions & 47 deletions packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v0/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,7 @@ use crate::data_contract::document_type::index::Index;
use crate::data_contract::document_type::index_level::IndexLevel;
use crate::data_contract::document_type::property::{DocumentProperty, DocumentPropertyType};
#[cfg(feature = "validation")]
use crate::data_contract::document_type::schema::{
byte_array_has_no_items_as_parent_validator, pattern_is_valid_regex_validator,
traversal_validator, validate_max_depth,
};
use crate::data_contract::document_type::schema::validate_max_depth;
use crate::data_contract::document_type::v0::DocumentTypeV0;
#[cfg(feature = "validation")]
use crate::data_contract::document_type::v0::StatelessJsonSchemaLazyValidator;
Expand Down Expand Up @@ -151,34 +148,12 @@ impl DocumentTypeV0 {
)
})?;

json_schema_validator.compile(&root_json_schema, platform_version)?;

// Validate against JSON Schema
DOCUMENT_META_SCHEMA_V0
.validate(&root_schema.try_to_validating_json().map_err(|e| {
ProtocolError::ConsensusError(
ConsensusError::BasicError(BasicError::ValueError(e.into())).into(),
)
})?)
.validate(&root_json_schema)
.map_err(|mut errs| ConsensusError::from(errs.next().unwrap()))?;

// TODO: Are we still aiming to use RE2 with linear time complexity to protect from ReDoS attacks?
// If not we can remove this validation
// Validate reg exp compatibility with RE2 and byteArray usage
let mut traversal_validator_result = traversal_validator(
&root_schema,
&[
pattern_is_valid_regex_validator,
byte_array_has_no_items_as_parent_validator,
],
platform_version,
)?;

if !traversal_validator_result.is_valid() {
let error = traversal_validator_result.errors.remove(0);

return Err(ProtocolError::ConsensusError(Box::new(error)));
}
json_schema_validator.compile(&root_json_schema, platform_version)?;
}

// This has already been validated, but we leave the map_err here for consistency
Expand Down Expand Up @@ -474,20 +449,6 @@ impl DocumentTypeV0 {
.transpose()?
.unwrap_or(SecurityLevel::HIGH);

#[cfg(feature = "validation")]
if full_validation && security_level_requirement == SecurityLevel::MASTER {
return Err(ConsensusError::BasicError(
BasicError::InvalidDocumentTypeRequiredSecurityLevelError(
InvalidDocumentTypeRequiredSecurityLevelError::new(
security_level_requirement,
data_contract_id,
name.to_string(),
),
),
)
.into());
}

let requires_identity_encryption_bounded_key = schema
.get_optional_integer::<u8>(property_names::REQUIRES_IDENTITY_ENCRYPTION_BOUNDED_KEY)
.map_err(consensus_or_protocol_value_error)?
Expand Down Expand Up @@ -584,11 +545,12 @@ fn insert_values(
));
}
}
// TODO: Contract indices and new encoding format don't support arrays
// but we still can use them as document fields with current cbor encoding
// This is a temporary workaround to bring back v0.22 behavior and should be
// replaced with a proper array support in future versions
None => DocumentPropertyType::Array(ArrayItemType::Boolean),
// TODO: Update when arrays are implemented
None => {
return Err(DataContractError::InvalidContractStructure(
"only byte arrays are supported now".to_string(),
));
}
};

document_properties.insert(
Expand Down
Loading

0 comments on commit bff4b65

Please sign in to comment.