Fix usage of NegateSignatureS in script_tests #5784

pull sdaftuar wants to merge 2 commits into bitcoin:master from sdaftuar:fix-test-negatesigS changing 3 files +20 −4

sdaftuar commented at 6:30 PM on February 10, 2015: member

NegateSignatureS is called with a signature which has not yet had the hashtype appended to it, yet the function assumed a hashtype was there and was incorrectly saving and then appending the last byte of its input (ie the last byte of the original S instead of a hashtype).

Only one pair of the test scripts was triggering this bug ("P2PK with high S"), which was actually causing the invalid version of the test to fail in the wrong place -- it was failing in the IsValidSignatureEncoding function rather than further down in IsLowDERSignature where the S value is checked. Fixing NegateSignatureS causes this test to change so that IsLowDERSignature is now being tested as I believe was intended.

Fixing that test resulted in there no longer being code coverage for the check in IsValidSignatureEncoding relating to extra bytes after the S, so the second commit here adds a pair of tests to exercise that check.

Fix NegateSignatureS to not duplicate last byte of S

NegateSignatureS is called with a signature without a hashtype, so
do not save the last byte and append it after S negation.

Updates the two tests which were affected by this bug.

6f50dbd2fd

Add test for DER-encoding edge case

The fix to NegateSignatureS caused a test which had been failing
in IsValidSignatureEncoding to then fail in IsLowDERSignature.
Add new test so the original check remains exercised.

78c6bedb9c

sipa commented at 6:18 AM on February 14, 2015: member

ACK
gmaxwell commented at 2:37 AM on February 17, 2015: contributor

ACK
laanwj merged this on Feb 18, 2015
laanwj closed this on Feb 18, 2015
laanwj referenced this in commit e3a3cd7a28 on Feb 18, 2015
random-zebra referenced this in commit 54190c58d9 on Sep 15, 2021
DrahtBot locked this on Feb 15, 2022

Contributors

sdaftuar

sipa

gmaxwell