Page MenuHomePhabricator

fix scriptSig analysis in sign.cpp
ClosedPublic

Authored by markblundeberg on Sun, Jan 12, 05:45.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Commits
rABCc95c9b9fa75c: fix scriptSig analysis in sign.cpp
Summary

This is used for analyzing partially unsigned transactions, and exhibits
weird behaviour where it allows any opcodes (including CPU-intensive ones)
to be run, and then it analyses the aftermath.

Also, this is a misuse of MANDATORY_SCRIPT_VERIFY_FLAGS as it here we
shouldn't care about how invalid transactions are sometimes banned/not
banned in mempool acceptance. In Core they have STRICTENC here which
is pretty weird, since it doesn't really have any effect (invalidly
encoded signatures still get through since they are just pushed).

This changes the evaluation to only proceed if the script is properly
push-only, and uses neutral flags to build a stack out of the push
opcodes.

(In principle we could enforce MINIMALDATA, but, it doesn't hurt to
allow nonstandard pushes since all the pushes will get canonicalized by
PushAll after signing.)

Test Plan

ninja check-all

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

markblundeberg created this revision.Sun, Jan 12, 05:45
Herald added a reviewer: Restricted Project. · View Herald TranscriptSun, Jan 12, 05:45
deadalnix accepted this revision.Sun, Jan 12, 15:13
This revision is now accepted and ready to land.Sun, Jan 12, 15:13
markblundeberg retitled this revision from fic scriptSig analysis in sign.cpp to fix scriptSig analysis in sign.cpp.Mon, Jan 13, 00:08