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.

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

Assertions not triggering with -O and test suite failure #44

Open
nh2 opened this issue May 19, 2019 · 2 comments
Open

Assertions not triggering with -O and test suite failure #44

nh2 opened this issue May 19, 2019 · 2 comments

Comments

@nh2
Copy link

nh2 commented May 19, 2019

Hi,

version 0.11.4.16.

When you test with ./Setup build test:test-evp-base64 --ghc-option=-O0 or in ghci, then a test failure occurs (I've inserted some more stack traces and prints for your convenience):

in assertFunction: print (n, x, y, r)
("decodeBase64BS","YWJjZGVmZ2hpams=\n","abcdefghijk","decodeBlock "YWJjZGVmZ2hpams=\n"
*** Exception: Assertion failed
CallStack (from HasCallStack):
  assert, called at ./OpenSSL/EVP/Base64.hs:111:45 in main:OpenSSL.EVP.Base64
  decodeBlock, called at ./OpenSSL/EVP/Base64.hs:130:18 in main:OpenSSL.EVP.Base64
  decodeBase64BS, called at Test/OpenSSL/EVP/Base64.hs:51:37 in main:Main

This assertion triggers: https://github.com/vshabanov/HsOpenSSL/blob/a9efae0b598b9499443721a1989055ca017fd01f/OpenSSL/EVP/Base64.hs#L109-L111

This is because in the test https://github.com/vshabanov/HsOpenSSL/blob/master/Test/OpenSSL/EVP/Base64.hs#L49-L58

the last test case, "YWJjZGVmZ2hpams=\n" has 17 chars, and 17 mod 4 is not 0.

This assertion failure usually goes completely unnoticed, because assertions are compiled away when -O is used (which is the default).

Are these assertions relevant for security or correctness?

If yes, then they should not be assertions, because assertions should not be used for control flow and input validation. Alternatively, -fno-ignore-asserts can be used.

@vshabanov
Copy link
Collaborator

Bad, bad code. The more I look at this code the more I want to deprecate it in favor of https://hackage.haskell.org/package/base64-bytestring

decodeBase64 "YWJjZGVmZ2hpams=\n" leads to segmentation fault (and decodeBase64 "YWJj\n" too)

decodeBase64BS fails with exception on 1,2,3 character inputs or any other invalid input (due to _DecodeBlock returning negative value)

decodeBase64LBS has segmentation fault on any input not divisible by 4 (due to infinite loop which must be prevented by assert).

Counting '=' is bad as well:
https://github.com/vshabanov/HsOpenSSL/blob/a9efae0b598b9499443721a1989055ca017fd01f/OpenSSL/EVP/Base64.hs#L119

OpenSSL seems to just decode 4-byte blocks (I can't understand what it does with newlines but it seems to stop on them). It can decode "AB==CD==EF==" and removing 6 bytes from output makes no sense.

Current plan:

  • these asserts must be converted to error
  • count '=' should be replaced to checking whether input ends on "=" or "=="
  • negative results of _DecodeBlockmust be handled with error
  • note about decoding of "AB==CD==" should be added
  • module should be deprecated in favor of base64-bytestring

@nh2
Copy link
Author

nh2 commented Jun 17, 2019

@vshabanov That plan sounds good to me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants