Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Transaction input/output versions are not bound to commitment signatures #4758

Closed
AaronFeickert opened this issue Sep 29, 2022 · 3 comments
Closed

Transaction input/output versions are not bound to commitment signatures #4758

AaronFeickert opened this issue Sep 29, 2022 · 3 comments
Assignees
@SWvheerden
Labels
A-base_node Area - The Tari base node executable and libraries A-security Area - Security related A-wallet Area - related to the wallet C-bug Category - fixes a bug, typically associated with an issue. W-consensus_breaking Warn - A change requiring a hard fork to be activated
Milestone
Stagenet Freeze

Comments

@AaronFeickert
Copy link
Collaborator

Transaction inputs and output versions are used when creating commitment signatures. However, a representation of the version is not bound to the signature challenge, which could be used in an attempted downgrade or mismatch attack.

It would be safer to bind the version to the signature challenge.
@agubarev agubarev self-assigned this Oct 26, 2022
@stringhandler stringhandler added C-bug Category - fixes a bug, typically associated with an issue. A-base_node Area - The Tari base node executable and libraries A-security Area - Security related W-consensus_breaking Warn - A change requiring a hard fork to be activated A-wallet Area - related to the wallet labels Nov 7, 2022
@stringhandler stringhandler added this to the Stagenet Freeze milestone Nov 7, 2022
@SWvheerden
Copy link
Collaborator

Script signatures do not bound to the version:

tari/base_layer/core/src/transactions/transaction_components/transaction_input.rs

Line 162 in 163dce0

pub fn build_script_challenge(

@SWvheerden
Copy link
Collaborator

same for output:

tari/base_layer/core/src/transactions/transaction_components/transaction_output.rs

Line 290 in 163dce0

let common = DomainSeparatedConsensusHasher::<TransactionHashDomain>::new("metadata_signature")

@agubarev agubarev assigned SWvheerden and unassigned agubarev Nov 30, 2022
@SWvheerden SWvheerden mentioned this issue Nov 30, 2022
Merged
stringhandler pushed a commit that referenced this issue Dec 8, 2022
@SWvheerden
fix!: input and output signatures (#4983)
a0f1d95 
Description
---
Script and metadata signature now commit to version
Make sure Gen block checks signatures
Reset  gen block

Motivation and Context
---
See: #4758

How Has This Been Tested?
---
Unit tests


BREAKING: This changes the challenge of both the script and metadata signatures
@AaronFeickert
Copy link
Collaborator Author

Fixed in PR 4983.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@SWvheerden SWvheerden

Labels
A-base_node Area - The Tari base node executable and libraries A-security Area - Security related A-wallet Area - related to the wallet C-bug Category - fixes a bug, typically associated with an issue. W-consensus_breaking Warn - A change requiring a hard fork to be activated
Projects
Tari Esme Testnet
Archived in project
Milestone
Stagenet Freeze
Development

No branches or pull requests
4 participants
@agubarev @SWvheerden @stringhandler @AaronFeickert