Skip to content

AVM: Catch any panic in edcsa verifying #4368

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

Merged
algorandskiy merged 3 commits into from
Jan 6, 2023
Merged

AVM: Catch any panic in edcsa verifying #4368

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9a89ce4
Catch any panic in edcsa verifying
jannotti Aug 7, 2022
2e2cec1
typo
jannotti Aug 9, 2022
d7c87d7
Merge remote-tracking branch 'origin/master' into go19-crypto-panics
cce Sep 6, 2022
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
59 changes: 35 additions & 24 deletions data/transactions/logic/eval.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3056,6 +3056,40 @@ var ecdsaVerifyCosts = []int{
Secp256r1: 2500,
}

// ecdsaVerify cheks a signature,
Comment thread
jannotti marked this conversation as resolved.
Outdated
func (cx *EvalContext) ecdsaVerify(curve EcdsaCurve, pkX, pkY []byte, msg []byte, sigR, sigS []byte) (result bool) {
// Go 1.19 panics on bad inputs. Catch it so that re can return false cleanly.
defer func() {
if recover() != nil {

@michaeldiamant michaeldiamant Nov 5, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving a note for when we return to the PR: I'm open to discussion, but initial my preference is to catch the specific failure condition rather than the catch-all presented by the PR.

Additionally - I opened an issue to track upgrading to Go v1.19 with a checklist item referencing this PR: #4759. Intent is to confirm we finalize the PR prior to completing a v1.19 upgrade.

result = false
}
}()

x := new(big.Int).SetBytes(pkX)
y := new(big.Int).SetBytes(pkY)

switch curve {
case Secp256k1:
signature := make([]byte, 0, len(sigR)+len(sigS))
signature = append(signature, sigR...)
signature = append(signature, sigS...)

pubkey := secp256k1.S256().Marshal(x, y)
return secp256k1.VerifySignature(pubkey, msg, signature)
case Secp256r1:
r := new(big.Int).SetBytes(sigR)
s := new(big.Int).SetBytes(sigS)

pubkey := ecdsa.PublicKey{
Curve: elliptic.P256(),
X: x,
Y: y,
}
return ecdsa.Verify(&pubkey, msg, r, s)
}
return false
}

func opEcdsaVerify(cx *EvalContext) error {
ecdsaCurve := EcdsaCurve(cx.program[cx.pc+1])
fs, ok := ecdsaCurveSpecByField(ecdsaCurve)
Expand Down Expand Up @@ -3083,30 +3117,7 @@ func opEcdsaVerify(cx *EvalContext) error {
return fmt.Errorf("the signed data must be 32 bytes long, not %d", len(msg))
}

x := new(big.Int).SetBytes(pkX)
y := new(big.Int).SetBytes(pkY)

var result bool
if fs.field == Secp256k1 {
signature := make([]byte, 0, len(sigR)+len(sigS))
signature = append(signature, sigR...)
signature = append(signature, sigS...)

pubkey := secp256k1.S256().Marshal(x, y)
result = secp256k1.VerifySignature(pubkey, msg, signature)
} else if fs.field == Secp256r1 {
r := new(big.Int).SetBytes(sigR)
s := new(big.Int).SetBytes(sigS)

pubkey := ecdsa.PublicKey{
Curve: elliptic.P256(),
X: x,
Y: y,
}
result = ecdsa.Verify(&pubkey, msg, r, s)
}

cx.stack[fifth] = boolToSV(result)
cx.stack[fifth] = boolToSV(cx.ecdsaVerify(fs.field, pkX, pkY, msg, sigR, sigS))
cx.stack = cx.stack[:fourth]
return nil
}
Expand Down