Page MenuHomePhabricator

fix scriptSig analysis in sign.cpp

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



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

(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

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

Event Timeline

This revision is now accepted and ready to land.Jan 12 2020, 15:13
markblundeberg retitled this revision from fic scriptSig analysis in sign.cpp to fix scriptSig analysis in sign.cpp.Jan 13 2020, 00:08