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.

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

fix!: fix opcode signatures #5966

Merged

Conversation

AaronFeickert
Copy link
Collaborator

@AaronFeickert AaronFeickert commented Nov 18, 2023

Description

Fixes opcode signatures to mitigate security issues.

Closes #5817.

Motivation and Context

Handling of signature-related opcodes introduces security issues related to signature forgery as described in #5817. This PR changes how signature-related message data is handled. Because of the new design, signature arithmetic support is removed.

How Has This Been Tested?

Existing tests pass or have been updated to reflect the new design.

What process can a PR reviewer use to test or verify this change?

Check that the new design matches the intent of the signature-related opcodes. Check that test modifications are correct.

BREAKING CHANGE: Changes how some opcodes are processed, which renders some existing scripts and transactions invalid.

@ghpbot-tari-project ghpbot-tari-project added P-acks_required Process - Requires more ACKs or utACKs P-reviews_required Process - Requires a review from a lead maintainer to be merged labels Nov 18, 2023
Copy link

github-actions bot commented Nov 18, 2023

Test Results (CI)

1 253 tests   1 253 ✔️  12m 23s ⏱️
     39 suites         0 💤
       1 files           0

Results for commit e760b4e.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Nov 18, 2023

Test Results (Integration tests)

31 tests   31 ✔️  14m 41s ⏱️
11 suites    0 💤
  2 files      0

Results for commit e760b4e.

♻️ This comment has been updated with latest results.

@ghpbot-tari-project ghpbot-tari-project removed the P-reviews_required Process - Requires a review from a lead maintainer to be merged label Nov 21, 2023
@SWvheerden SWvheerden merged commit dc26ca6 into tari-project:development Nov 21, 2023
14 checks passed
@AaronFeickert AaronFeickert deleted the opcode-signatures branch November 21, 2023 15:10
sdbondi pushed a commit to sdbondi/tari that referenced this pull request Nov 27, 2023
Description
---
Fixes opcode signatures to mitigate security issues.

Closes tari-project#5817.

Motivation and Context
---
Handling of signature-related opcodes introduces security issues related
to signature forgery as described in tari-project#5817. This PR changes how
signature-related message data is handled. Because of the new design,
signature arithmetic support is removed.

How Has This Been Tested?
---
Existing tests pass or have been updated to reflect the new design.

What process can a PR reviewer use to test or verify this change?
---
Check that the new design matches the intent of the signature-related
opcodes. Check that test modifications are correct.

BREAKING CHANGE: Changes how some opcodes are processed, which renders
some existing scripts and transactions invalid.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-acks_required Process - Requires more ACKs or utACKs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Signature opcode handling allows trivial forgery
3 participants