Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -68,17 +68,16 @@ impl AllowedAsMultiPartyAction for TokenSetPriceForDirectPurchaseTransitionV0 {
fn calculate_action_id(
&self,
owner_id: Identifier,
_platform_version: &PlatformVersion,
platform_version: &PlatformVersion,
) -> Result<Identifier, ProtocolError> {
let TokenSetPriceForDirectPurchaseTransitionV0 { base, price, .. } = self;

Ok(
TokenSetPriceForDirectPurchaseTransition::calculate_action_id_with_fields(
base.token_id().as_bytes(),
owner_id.as_bytes(),
base.identity_contract_nonce(),
price.as_ref(),
),
TokenSetPriceForDirectPurchaseTransition::calculate_action_id_with_fields(
base.token_id().as_bytes(),
owner_id.as_bytes(),
base.identity_contract_nonce(),
price.as_ref(),
platform_version,
)
}
}
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
use platform_value::Identifier;
use platform_version::version::PlatformVersion;
use crate::errors::ProtocolError;
use crate::prelude::IdentityNonce;
use crate::ProtocolError;
use crate::state_transition::batch_transition::batched_transition::multi_party_action::AllowedAsMultiPartyAction;
use crate::state_transition::batch_transition::token_base_transition::token_base_transition_accessors::TokenBaseTransitionAccessors;
use crate::state_transition::batch_transition::token_base_transition::TokenBaseTransition;
use crate::state_transition::batch_transition::token_set_price_for_direct_purchase_transition::TokenSetPriceForDirectPurchaseTransition;
use crate::state_transition::batch_transition::token_set_price_for_direct_purchase_transition::v0::v0_methods::TokenSetPriceForDirectPurchaseTransitionV0Methods;
use crate::state_transition::batch_transition::token_set_price_for_direct_purchase_transition::TokenSetPriceForDirectPurchaseTransition;
use crate::tokens::token_pricing_schedule::TokenPricingSchedule;
use crate::util::hash::hash_double;
use platform_value::Identifier;
use platform_version::version::PlatformVersion;

impl TokenBaseTransitionAccessors for TokenSetPriceForDirectPurchaseTransition {
fn base(&self) -> &TokenBaseTransition {
Expand Down Expand Up @@ -84,6 +84,39 @@ impl TokenSetPriceForDirectPurchaseTransition {
owner_id: &[u8; 32],
identity_contract_nonce: IdentityNonce,
price_per_token: Option<&TokenPricingSchedule>,
platform_version: &PlatformVersion,
) -> Result<Identifier, ProtocolError> {
match platform_version
.dpp
.token_versions
.token_set_price_action_id_version
{
0 => Ok(Self::calculate_action_id_with_fields_v0(
token_id,
owner_id,
identity_contract_nonce,
price_per_token,
)),
1 => Self::calculate_action_id_with_fields_v1(
token_id,
owner_id,
identity_contract_nonce,
price_per_token,
),
version => Err(ProtocolError::UnknownVersionMismatch {
method: "calculate_action_id_with_fields".to_string(),
known_versions: vec![0, 1],
received: version,
}),
}
}

/// v0: hashes only minimum_purchase_amount_and_price().1 (kept for backward compat).
fn calculate_action_id_with_fields_v0(
token_id: &[u8; 32],
owner_id: &[u8; 32],
identity_contract_nonce: IdentityNonce,
price_per_token: Option<&TokenPricingSchedule>,
) -> Identifier {
let mut bytes = b"action_token_set_price_for_direct_purchase".to_vec();
bytes.extend_from_slice(token_id);
Expand All @@ -100,4 +133,197 @@ impl TokenSetPriceForDirectPurchaseTransition {

hash_double(bytes).into()
}

/// v1: hashes the full serialized TokenPricingSchedule, preventing schedule swap.
fn calculate_action_id_with_fields_v1(
token_id: &[u8; 32],
owner_id: &[u8; 32],
identity_contract_nonce: IdentityNonce,
price_per_token: Option<&TokenPricingSchedule>,
) -> Result<Identifier, ProtocolError> {
let mut bytes = b"action_token_set_price_for_direct_purchase".to_vec();
bytes.extend_from_slice(token_id);
bytes.extend_from_slice(owner_id);
bytes.extend_from_slice(&identity_contract_nonce.to_be_bytes());
if let Some(price_per_token) = price_per_token {
let serialized = bincode::encode_to_vec(price_per_token, bincode::config::standard())
.map_err(|e| ProtocolError::EncodingError(e.to_string()))?;
bytes.extend_from_slice(&serialized);
}

Ok(hash_double(bytes).into())
}
}

#[cfg(test)]
mod tests {
use super::*;
use crate::state_transition::batch_transition::token_base_transition::v0::TokenBaseTransitionV0;
use crate::state_transition::batch_transition::token_set_price_for_direct_purchase_transition::TokenSetPriceForDirectPurchaseTransitionV0;
use platform_version::version::PlatformVersion;
use std::collections::BTreeMap;

fn make_transition(
price: Option<TokenPricingSchedule>,
) -> TokenSetPriceForDirectPurchaseTransition {
TokenSetPriceForDirectPurchaseTransition::V0(TokenSetPriceForDirectPurchaseTransitionV0 {
base: TokenBaseTransition::V0(TokenBaseTransitionV0 {
identity_contract_nonce: 1,
token_contract_position: 0,
data_contract_id: Identifier::new([1u8; 32]),
token_id: Identifier::new([2u8; 32]),
using_group_info: None,
}),
price,
public_note: None,
})
}

#[test]
fn different_set_prices_with_same_minimum_produce_different_ids() {
// This was the vulnerability: two SetPrices schedules with the same
// minimum-tier price but different higher tiers produced identical
// action_ids when only minimum_purchase_amount_and_price().1 was hashed.
let owner_id = Identifier::new([3u8; 32]);

let t_cheap = make_transition(Some(TokenPricingSchedule::SetPrices(BTreeMap::from([
(1, 100),
(10, 800),
]))));
let t_expensive = make_transition(Some(TokenPricingSchedule::SetPrices(BTreeMap::from([
(1, 100),
(10, 9999),
]))));

let id_cheap = t_cheap
.calculate_action_id(owner_id, PlatformVersion::latest())
.expect("expected action id");
let id_expensive = t_expensive
.calculate_action_id(owner_id, PlatformVersion::latest())
.expect("expected action id");

assert_ne!(
id_cheap, id_expensive,
"different pricing schedules with same minimum price must produce different action_ids"
);
}

#[test]
fn single_price_and_set_prices_with_same_minimum_produce_different_ids() {
// SinglePrice(100) and SetPrices({1: 100}) both have
// minimum_purchase_amount_and_price() == (1, 100), but they are
// semantically different schedules.
let owner_id = Identifier::new([3u8; 32]);

let t_single = make_transition(Some(TokenPricingSchedule::SinglePrice(100)));
let t_set = make_transition(Some(TokenPricingSchedule::SetPrices(BTreeMap::from([(
1, 100,
)]))));

let id_single = t_single
.calculate_action_id(owner_id, PlatformVersion::latest())
.expect("expected action id");
let id_set = t_set
.calculate_action_id(owner_id, PlatformVersion::latest())
.expect("expected action id");

assert_ne!(
id_single, id_set,
"SinglePrice and SetPrices with same minimum must produce different action_ids"
);
}

#[test]
fn identical_schedules_produce_same_id() {
let owner_id = Identifier::new([3u8; 32]);

let t1 = make_transition(Some(TokenPricingSchedule::SetPrices(BTreeMap::from([
(1, 100),
(10, 800),
]))));
let t2 = make_transition(Some(TokenPricingSchedule::SetPrices(BTreeMap::from([
(1, 100),
(10, 800),
]))));

let pv = PlatformVersion::latest();
assert_eq!(
t1.calculate_action_id(owner_id, pv)
.expect("expected action id"),
t2.calculate_action_id(owner_id, pv)
.expect("expected action id"),
"identical pricing schedules must produce the same action_id"
);
}

#[test]
fn none_price_produces_different_id_from_some_price() {
let owner_id = Identifier::new([3u8; 32]);
let pv = PlatformVersion::latest();

let t_none = make_transition(None);
let t_some = make_transition(Some(TokenPricingSchedule::SinglePrice(100)));

assert_ne!(
t_none
.calculate_action_id(owner_id, pv)
.expect("expected action id"),
t_some
.calculate_action_id(owner_id, pv)
.expect("expected action id"),
"None price and Some price must produce different action_ids"
);
}

#[test]
fn v0_same_minimum_produces_same_id_vulnerability() {
// Documents the v0 vulnerability: different schedules with the same
// minimum-tier price produce identical action_ids.
let owner_id = Identifier::new([3u8; 32]);
let pv = PlatformVersion::first();

let t_cheap = make_transition(Some(TokenPricingSchedule::SetPrices(BTreeMap::from([
(1, 100),
(10, 800),
]))));
let t_expensive = make_transition(Some(TokenPricingSchedule::SetPrices(BTreeMap::from([
(1, 100),
(10, 9999),
]))));

let id_cheap = t_cheap
.calculate_action_id(owner_id, pv)
.expect("expected action id");
let id_expensive = t_expensive
.calculate_action_id(owner_id, pv)
.expect("expected action id");

// v0: these are EQUAL -- the vulnerability
assert_eq!(
id_cheap, id_expensive,
"v0 should produce the same action_id for different schedules with same min price"
);
}

#[test]
fn v0_and_v1_produce_different_ids_for_same_input() {
let owner_id = Identifier::new([3u8; 32]);

let t = make_transition(Some(TokenPricingSchedule::SetPrices(BTreeMap::from([
(1, 100),
(10, 800),
]))));

let id_v0 = t
.calculate_action_id(owner_id, PlatformVersion::first())
.expect("expected action id");
let id_v1 = t
.calculate_action_id(owner_id, PlatformVersion::latest())
.expect("expected action id");

assert_ne!(
id_v0, id_v1,
"v0 and v1 should produce different action_ids for the same schedule"
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,8 @@ pub struct DPPTokenVersions {
/// v0: uses only the u8 discriminant of the config change item (vulnerable to value swap)
/// v1: includes the full serialized config change item in the hash
pub token_config_update_action_id_version: FeatureVersion,
/// Version for the set-price-for-direct-purchase action_id calculation.
/// v0: uses only minimum_purchase_amount_and_price().1 (vulnerable to schedule swap)
/// v1: includes the full serialized TokenPricingSchedule in the hash
pub token_set_price_action_id_version: FeatureVersion,
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@ pub const TOKEN_VERSIONS_V1: DPPTokenVersions = DPPTokenVersions {
identity_token_status_default_structure_version: 0,
token_contract_info_default_structure_version: 0,
token_config_update_action_id_version: 0,
token_set_price_action_id_version: 0,
};
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@ pub const TOKEN_VERSIONS_V2: DPPTokenVersions = DPPTokenVersions {
identity_token_status_default_structure_version: 0,
token_contract_info_default_structure_version: 0,
token_config_update_action_id_version: 1,
token_set_price_action_id_version: 1,
};
2 changes: 1 addition & 1 deletion packages/rs-platform-version/src/version/v12.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ pub const PLATFORM_V12: PlatformVersion = PlatformVersion {
document_versions: DOCUMENT_VERSIONS_V3,
identity_versions: IDENTITY_VERSIONS_V1,
voting_versions: VOTING_VERSION_V2,
token_versions: TOKEN_VERSIONS_V2, // fixes issue with token config update action id
token_versions: TOKEN_VERSIONS_V2, // fixes action_id vote-swap for config update + set price
asset_lock_versions: DPP_ASSET_LOCK_VERSIONS_V1,
methods: DPP_METHOD_VERSIONS_V2,
factory_versions: DPP_FACTORY_VERSIONS_V1,
Expand Down
Loading