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.

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

Jump to bottom

Add known-to-fail counterexamples to Bech32 string corruption tests. #328

Merged
merged 3 commits into from
May 28, 2019
Merged

Add known-to-fail counterexamples to Bech32 string corruption tests. #328

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5beb548
Increase maximum number of runs for Bech32 adjacent character swap test.
jonathanknowles May 28, 2019
536ed98
Allow a known-to-fail counterexample within the Bech32 character omis…
jonathanknowles May 28, 2019
3a49b99
Allow a known-to-fail counterexample within the Bech32 character inse…
jonathanknowles May 28, 2019
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
26 changes: 23 additions & 3 deletions lib/bech32/test/Codec/Binary/Bech32Spec.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ import Test.QuickCheck
, counterexample
, elements
, property
, withMaxSuccess
, (.&&.)
, (.||.)
, (===)
Expand Down Expand Up @@ -125,7 +126,7 @@ spec = do
describe "Decoding a corrupted string should fail" $ do

it "Decoding fails when an adjacent pair of characters is swapped." $
property $ \s -> do
property $ withMaxSuccess 10000 $ \s -> do
let originalString = getValidBech32String s
index <- choose (0, T.length originalString - 2)
let prefix = T.take index originalString
Expand All @@ -147,7 +148,7 @@ spec = do
(Bech32.decode corruptedString `shouldSatisfy` isLeft)

it "Decoding fails when a character is omitted." $
property $ \s -> do
property $ withMaxSuccess 10000 $ \s -> do
let originalString = getValidBech32String s
index <- choose (0, T.length originalString - 1)
let char = T.index originalString index
Expand All @@ -163,9 +164,19 @@ spec = do
(T.length corruptedString === T.length originalString - 1)
.&&.
(Bech32.decode corruptedString `shouldSatisfy` isLeft)
.||.
-- In the case where the tail of a valid Bech32 string is
-- composed of one or more consecutive 'q' characters
-- followed by a single 'p' character, omitting any or all
-- of the 'q' characters will still result in a valid
-- Bech32 string:
(T.length suffix > 0
&& T.last suffix == 'p'
&& char == 'q'
&& T.all (== 'q') (T.dropEnd 1 suffix))
Copy link
Member

Choose a reason for hiding this comment

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

What's the rationale here 🤔 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question. I'm currently updating the issue with a longer explanation. Will let you know when I've finished writing. :)

Copy link
Member Author

Choose a reason for hiding this comment

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

In our current test case, we assume that deleting a character from a valid Bech32 string will create an invalid Bech32 string.

I think this is a fair assumption, as Bech32 was designed with the specific goal of collision resistance. When entering one Bech32 string, it's supposed to be very hard to collide with another Bech32 string just by making a small typo.

Experimental testing shows that there is at least one known counterexample to this law: when a Bech32 string ends in qp, it's possible to delete a single q character and still produce a valid Bech32 string. This works for any number of q characters.

The fact that it's possible to produce a collision just by deleting a single character comes as a surprise.

The above change adjusts the test case to account for this counterexample.


it "Decoding fails when a character is inserted." $
property $ \s c -> do
property $ withMaxSuccess 10000 $ \s c -> do
let originalString = getValidBech32String s
let char = getDataChar c
index <- choose (0, T.length originalString - 1)
Expand All @@ -181,6 +192,15 @@ spec = do
(T.length corruptedString === T.length originalString + 1)
.&&.
(Bech32.decode corruptedString `shouldSatisfy` isLeft)
.||.
-- In the case where the last character of a valid Bech32
-- string is the character 'p', inserting any number of
-- consecutive 'q' characters immediately before the 'p'
-- will still result in a valid Bech32 string.
(T.length suffix > 0
&& T.last suffix == 'p'
&& char == 'q'
&& T.all (== 'q') (T.dropEnd 1 suffix))

it "Decoding fails when a single character is mutated." $
property $ \s c -> do
Expand Down