From f0a49645114964c4f447d5135ee335e8f056ade3 Mon Sep 17 00:00:00 2001 From: Joris Dral Date: Mon, 10 Nov 2025 15:24:56 +0100 Subject: [PATCH] Fix a bug in public-key `encrypt` and `decrypt` functions We now use upper bounds on output buffer sizes as provided by the Botan C FFI to allocate buffers, rather than querying the encryption/decryption fuctions with a null pointer (see https://github.com/randombit/botan/issues/5154), which are arguably suffering from a bug. Using the upper bounds was already advised by the Botan C FFI anyway. --- botan-low/CHANGELOG.md | 3 ++ botan-low/src/Botan/Low/PubKey/Decrypt.hs | 25 ++++++---- botan-low/src/Botan/Low/PubKey/Encrypt.hs | 47 ++++++++++--------- .../test/Test/Botan/Low/PubKey/Decrypt.hs | 5 +- .../test/Test/Botan/Low/PubKey/Encrypt.hs | 6 --- 5 files changed, 46 insertions(+), 40 deletions(-) diff --git a/botan-low/CHANGELOG.md b/botan-low/CHANGELOG.md index d9e3583..b756d1f 100644 --- a/botan-low/CHANGELOG.md +++ b/botan-low/CHANGELOG.md @@ -39,6 +39,9 @@ "Lion" and "Cascade" ciphers respectively. * PATCH: fix an "address out of bounds" bug in `blockCipherEncryptBlocks` and `blockCipherDecryptBlocks` that occasionally caused segfaults. +* PATCH: fix an "insufficient buffer space" bug in + `Botan.Low.PubKey.Encrypt.encrypt` and `Botan.Low.PubKey.Decrypt.decrypt`. See + PR [#79](https://github.com/haskell-cryptography/botan/pull/79). ## 0.0.2.0 -- 2025-09-17 diff --git a/botan-low/src/Botan/Low/PubKey/Decrypt.hs b/botan-low/src/Botan/Low/PubKey/Decrypt.hs index 4d21900..ea14575 100644 --- a/botan-low/src/Botan/Low/PubKey/Decrypt.hs +++ b/botan-low/src/Botan/Low/PubKey/Decrypt.hs @@ -22,11 +22,13 @@ module Botan.Low.PubKey.Decrypt ( ) where import Botan.Bindings.PubKey.Decrypt - +import Botan.Low.Error (throwBotanIfNegative_) import Botan.Low.Make import Botan.Low.Prelude import Botan.Low.PubKey import Botan.Low.Remake +import qualified Data.ByteString as BS +import qualified Data.ByteString.Internal as BSI -- /* -- * Public Key Decryption @@ -61,15 +63,22 @@ decryptOutputLength -> IO Int -- ^ __ptext_len__ decryptOutputLength = mkGetSize_csize withDecrypt botan_pk_op_decrypt_output_length -decrypt - :: Decrypt -- ^ __op__ - -> ByteString -- ^ __ciphertext__ - -> IO ByteString -- ^ __plaintext__ -decrypt dec ctext = withDecrypt dec $ \ decPtr -> do - asBytesLen ctext $ \ ctextPtr ctextLen -> do - allocBytesQuerying $ \ outPtr szPtr -> botan_pk_op_decrypt +decrypt :: + Decrypt -- ^ __op__ + -> ByteString -- ^ __ciphertext__ + -> IO ByteString -- ^ __plaintext__ +decrypt dec ctext = + withDecrypt dec $ \ decPtr -> + asBytesLen ctext $ \ ctextPtr ctextLen -> + alloca $ \szPtr -> do + sz <- decryptOutputLength dec (BS.length ctext) + poke szPtr (fromIntegral sz) + BSI.createUptoN sz $ \outPtr -> do + throwBotanIfNegative_ $ + botan_pk_op_decrypt decPtr outPtr szPtr (ConstPtr ctextPtr) ctextLen + fromIntegral <$> peek szPtr diff --git a/botan-low/src/Botan/Low/PubKey/Encrypt.hs b/botan-low/src/Botan/Low/PubKey/Encrypt.hs index 17572f7..375a6b2 100644 --- a/botan-low/src/Botan/Low/PubKey/Encrypt.hs +++ b/botan-low/src/Botan/Low/PubKey/Encrypt.hs @@ -22,12 +22,14 @@ module Botan.Low.PubKey.Encrypt ( ) where import Botan.Bindings.PubKey.Encrypt - +import Botan.Low.Error (throwBotanIfNegative_) import Botan.Low.Make import Botan.Low.Prelude import Botan.Low.PubKey import Botan.Low.Remake import Botan.Low.RNG +import qualified Data.ByteString as BS +import qualified Data.ByteString.Internal as BSI -- /* -- * Public Key Encryption @@ -62,24 +64,25 @@ encryptOutputLength -> IO Int -- ^ __ctext_len__ encryptOutputLength = mkGetSize_csize withEncrypt botan_pk_op_encrypt_output_length --- NOTE: This properly takes advantage of szPtr, queries the buffer size - do this elsewhere --- NOTE: SM2 take a hash instead of a padding, and encrypt fails with InsufficientBufferSpace --- if sm2p256v1 is not used as the curve when creating the key (but creating the key and --- the encryption context do not fail) --- This implies that encryptOutputLength may be wrong or hardcoded for SM2 or that we --- are not supposed to use curves other than sm2p256v1 - this needs investigating -encrypt - :: Encrypt -- ^ __op__ - -> RNG -- ^ __rng__ - -> ByteString -- ^ __plaintext[]__ - -> IO ByteString -- ^ __ciphertext[]__ -encrypt enc rng ptext = withEncrypt enc $ \ encPtr -> do - withRNG rng $ \ botanRNG -> do - asBytesLen ptext $ \ ptextPtr ptextLen -> do - allocBytesQuerying $ \ outPtr szPtr -> botan_pk_op_encrypt - encPtr - botanRNG - outPtr - szPtr - ptextPtr - ptextLen +encrypt :: + Encrypt -- ^ __op__ + -> RNG -- ^ __rng__ + -> ByteString -- ^ __plaintext[]__ + -> IO ByteString -- ^ __ciphertext[]__ +encrypt enc rng ptext = + withEncrypt enc $ \ encPtr -> + withRNG rng $ \ botanRNG -> + asBytesLen ptext $ \ ptextPtr ptextLen -> + alloca $ \szPtr -> do + sz <- encryptOutputLength enc (BS.length ptext) + poke szPtr (fromIntegral sz) + BSI.createUptoN sz $ \outPtr -> do + throwBotanIfNegative_ $ + botan_pk_op_encrypt + encPtr + botanRNG + outPtr + szPtr + ptextPtr + ptextLen + fromIntegral <$> peek szPtr diff --git a/botan-low/test/Test/Botan/Low/PubKey/Decrypt.hs b/botan-low/test/Test/Botan/Low/PubKey/Decrypt.hs index 504ca92..07516c9 100644 --- a/botan-low/test/Test/Botan/Low/PubKey/Decrypt.hs +++ b/botan-low/test/Test/Botan/Low/PubKey/Decrypt.hs @@ -18,15 +18,12 @@ tests = do specs <- testSpec "spec_decrypt" spec_decrypt pure $ testGroup "Test.Botan.Low.PubKey.Decrypt" [ specs - -- TODO: temporarily disabled because the test suite fails. See issue - -- #33. - | False ] pks :: [(ByteString, ByteString, ByteString)] pks = [ ("RSA", "2048", "PKCS1v15") - , ("SM2", "sm2p256v1", "SHA-256") -- NOTE: Decrypt fails with InsufficientBufferSpace + , ("SM2", "sm2p256v1", "SHA-256") -- NOTE: SM2 takes a hash rather than a padding , ("ElGamal", "modp/ietf/1024", "PKCS1v15") ] diff --git a/botan-low/test/Test/Botan/Low/PubKey/Encrypt.hs b/botan-low/test/Test/Botan/Low/PubKey/Encrypt.hs index a2bb6b7..c3a5a1b 100644 --- a/botan-low/test/Test/Botan/Low/PubKey/Encrypt.hs +++ b/botan-low/test/Test/Botan/Low/PubKey/Encrypt.hs @@ -17,14 +17,8 @@ tests = do specs <- testSpec "spec_encrypt" spec_encrypt pure $ testGroup "Test.Botan.Low.PubKey.Encrypt" [ specs - -- TODO: temporarily disabled because the test suite fails. See issue - -- #33. - | False ] --- NOTE: SM2 encrypt fails with InsufficientBufferSpace unless sm2p256v1 is used as the --- curve when creating the key (but creating the key and the encryption context do not fail) - pks :: [(ByteString, ByteString, ByteString)] pks = [ ("RSA", "2048", "PKCS1v15")