Skip to content

Commit

Permalink
Avoid overflows on 32-bit systems (#677)
Browse files Browse the repository at this point in the history
* Avoid overflows in Secure Cell

Themis Core C API works with buffer sizes expressed as "size_t" while
in Go lengths are expressed as "int". Themis containers can typically
contain up to 4 GB of data with internal length fields using "uint32_t".

On typical 64-bit systems this does not cause overflows since uint32_t
fits into both Go's int and C's size_t. However, on 32-bit system this
can cause overflows. There, size_t is unsigned 32-bit value identical to
uint32_t while int is 32-bit signed value, so the size may not fit into
Go's size range.

We can't do anything about that. On 32-bit systems the buffer sizes are
typically limited to 2 GB anyway due to the way memory is distributed.
However, if the overflow happens, Go will panic when trying to allocate
(effectively) negatively-sized arrays. We should return an error instead.

Add size checks before casting "C.size_t" into "int" and return an error
if the size will overflow. Do this for all API, both new and old.

Normally, Themis is not used to encrypt real 2+ GB messages, but this
condition can easily happen if the data has been corrupted where the
length field is stored. We don't want this to be a source of DOS attacks.

* Reenable tests for corrupted data

The panic condition has been originally detected by a couple of tests
for Secure Cell's Token Protect mode which has the stars properly
aligned for the issue to be visible. Now that the issue is fixed, we can
enable these tests for 32-bit machines again.

* Avoid overflows in Secure Compartor
* Avoid overflows in key generation
* Avoid overflows in Secure Message
* Avoid overflows in Secure Session

Just like Secure Cell, add more checks to other cryptosystems as well.
Unfortunately, we have to duplicate the size check utility. GoThemis
does not have a common utility module, and even if it did, it would not
work due to the way CGo is implemented ("C.size_t" is a distinct type in
different modules).

(cherry picked from commit 8b83a71)
  • Loading branch information
ilammy committed Jul 21, 2020
1 parent e078f1e commit 66b6727
Show file tree
Hide file tree
Showing 12 changed files with 117 additions and 20 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@

Changes that are currently in development and have not been released yet.

_Code:_

- **Go**

- Fixed panics on 32-bit systems when processing corrupted data ([#677](https://github.com/cossacklabs/themis/pull/677)).

## [0.13.0](https://github.com/cossacklabs/themis/releases/tag/0.13.0), July 8th 2020

**TL;DR:**
Expand Down
17 changes: 16 additions & 1 deletion gothemis/cell/cell.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,9 @@ static bool decrypt(const void *key, size_t key_len, const void *prot, size_t pr
*/
import "C"
import (
"github.com/cossacklabs/themis/gothemis/errors"
"unsafe"

"github.com/cossacklabs/themis/gothemis/errors"
)

// Errors returned by Secure Cell.
Expand All @@ -185,6 +186,7 @@ var (
ErrMissingMessage = errors.NewWithCode(errors.InvalidParameter, "empty message for Secure Cell")
ErrMissingToken = errors.NewWithCode(errors.InvalidParameter, "authentication token is required in Token Protect mode")
ErrMissingContext = errors.NewWithCode(errors.InvalidParameter, "associated context is required in Context Imprint mode")
ErrOverflow = errors.NewWithCode(errors.NoMemory, "Secure Cell cannot allocate enough memory")
)

// Secure Cell operation mode.
Expand Down Expand Up @@ -225,6 +227,13 @@ func missing(data []byte) bool {
return data == nil || len(data) == 0
}

// C returns sizes as size_t but Go expresses buffer lengths as int.
// Make sure that all sizes are representable in Go and there is no overflows.
func sizeOverflow(n C.size_t) bool {
const maxInt = int(^uint(0) >> 1)
return n > C.size_t(maxInt)
}

// Protect encrypts or signs data with optional user context (depending on the Cell mode).
func (sc *SecureCell) Protect(data []byte, context []byte) ([]byte, []byte, error) {
if (sc.mode < ModeSeal) || (sc.mode > ModeContextImprint) {
Expand Down Expand Up @@ -266,6 +275,9 @@ func (sc *SecureCell) Protect(data []byte, context []byte) ([]byte, []byte, erro
&addLen)) {
return nil, nil, errors.New("Failed to get output size")
}
if sizeOverflow(encLen) || sizeOverflow(addLen) {
return nil, nil, ErrOverflow
}

var addData []byte
var add unsafe.Pointer
Expand Down Expand Up @@ -345,6 +357,9 @@ func (sc *SecureCell) Unprotect(protectedData []byte, additionalData []byte, con
&decLen)) {
return nil, errors.New("Failed to get output size")
}
if sizeOverflow(decLen) {
return nil, ErrOverflow
}

decData := make([]byte, decLen, decLen)
if !bool(C.decrypt(unsafe.Pointer(&sc.key[0]),
Expand Down
6 changes: 6 additions & 0 deletions gothemis/cell/context_imprint.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,9 @@ func encryptContextImprint(symmetricKey, plaintext, userContext, ciphertext []by
bytesData(ciphertext),
&ciphertextLength,
)
if sizeOverflow(ciphertextLength) {
return 0, errors.NoMemory
}
return int(ciphertextLength), errors.ThemisErrorCode(err)
}

Expand All @@ -153,5 +156,8 @@ func decryptContextImprint(symmetricKey, ciphertext, userContext, plaintext []by
bytesData(plaintext),
&plaintextLength,
)
if sizeOverflow(plaintextLength) {
return 0, errors.NoMemory
}
return int(plaintextLength), errors.ThemisErrorCode(err)
}
6 changes: 6 additions & 0 deletions gothemis/cell/seal.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,9 @@ func encryptSeal(symmetricKey, plaintext, userContext, ciphertext []byte) (int,
bytesData(ciphertext),
&ciphertextLength,
)
if sizeOverflow(ciphertextLength) {
return 0, errors.NoMemory
}
return int(ciphertextLength), errors.ThemisErrorCode(err)
}

Expand All @@ -152,5 +155,8 @@ func decryptSeal(symmetricKey, ciphertext, userContext, plaintext []byte) (int,
bytesData(plaintext),
&plaintextLength,
)
if sizeOverflow(plaintextLength) {
return 0, errors.NoMemory
}
return int(plaintextLength), errors.ThemisErrorCode(err)
}
6 changes: 6 additions & 0 deletions gothemis/cell/seal_pw.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,9 @@ func encryptSealWithPassphrase(passphrase string, plaintext, userContext, cipher
bytesData(ciphertext),
&ciphertextLength,
)
if sizeOverflow(ciphertextLength) {
return 0, errors.NoMemory
}
return int(ciphertextLength), errors.ThemisErrorCode(err)
}

Expand All @@ -141,5 +144,8 @@ func decryptSealWithPassphrase(passphrase string, ciphertext, userContext, plain
bytesData(plaintext),
&plaintextLength,
)
if sizeOverflow(plaintextLength) {
return 0, errors.NoMemory
}
return int(plaintextLength), errors.ThemisErrorCode(err)
}
6 changes: 6 additions & 0 deletions gothemis/cell/token_protect.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,9 @@ func encryptTokenProtect(symmetricKey, plaintext, userContext, ciphertext, authT
bytesData(ciphertext),
&ciphertextLength,
)
if sizeOverflow(ciphertextLength) || sizeOverflow(authTokenLength) {
return 0, 0, errors.NoMemory
}
return int(ciphertextLength), int(authTokenLength), errors.ThemisErrorCode(err)
}

Expand All @@ -165,5 +168,8 @@ func decryptTokenProtect(symmetricKey, ciphertext, authToken, userContext, plain
bytesData(plaintext),
&plaintextLength,
)
if sizeOverflow(plaintextLength) {
return 0, errors.NoMemory
}
return int(plaintextLength), errors.ThemisErrorCode(err)
}
14 changes: 0 additions & 14 deletions gothemis/cell/token_protect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -359,13 +359,6 @@ func TestTokenProtectDetectExtendedData(t *testing.T) {
}

func TestTokenProtectDetectCorruptedToken(t *testing.T) {
// FIXME(ilammy, 2020-05-25): avoid capacity allocation panics (T1648)
// This tests panics on 32-bit architectures due to "int" overflow.
// The implementation needs to check for "int" range when casting "C.size_t".
if uint64(^uint(0)) == uint64(^uint32(0)) {
t.Skip("avoid panic on 32-bit machines")
}

key, err := keys.NewSymmetricKey()
if err != nil {
t.Fatal("cannot generate symmetric key", err)
Expand Down Expand Up @@ -449,13 +442,6 @@ func TestTokenProtectDetectExtendedToken(t *testing.T) {
}

func TestTokenProtectSwapDataAndToken(t *testing.T) {
// FIXME(ilammy, 2020-05-25): avoid capacity allocation panics (T1648)
// This tests panics on 32-bit architectures due to "int" overflow.
// The implementation needs to check for "int" range when casting "C.size_t".
if uint64(^uint(0)) == uint64(^uint32(0)) {
t.Skip("avoid panic on 32-bit machines")
}

key, err := keys.NewSymmetricKey()
if err != nil {
t.Fatal("cannot generate symmetric key", err)
Expand Down
17 changes: 16 additions & 1 deletion gothemis/compare/compare.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,10 @@ static int compare_result(void *ctx)
*/
import "C"
import (
"github.com/cossacklabs/themis/gothemis/errors"
"runtime"
"unsafe"

"github.com/cossacklabs/themis/gothemis/errors"
)

// Secure comparison result.
Expand All @@ -78,6 +79,7 @@ const (
var (
ErrMissingSecret = errors.NewWithCode(errors.InvalidParameter, "empty secret for Secure Comparator")
ErrMissingData = errors.NewWithCode(errors.InvalidParameter, "empty comparison message for Secure Comparator")
ErrOverflow = errors.NewWithCode(errors.NoMemory, "Secure Comparator cannot allocate enough memory")
)

// SecureCompare is an interactive protocol for two parties that compares whether
Expand All @@ -90,6 +92,13 @@ func finalize(sc *SecureCompare) {
sc.Close()
}

// C returns sizes as size_t but Go expresses buffer lengths as int.
// Make sure that all sizes are representable in Go and there is no overflows.
func sizeOverflow(n C.size_t) bool {
const maxInt = int(^uint(0) >> 1)
return n > C.size_t(maxInt)
}

// New prepares a new secure comparison.
func New() (*SecureCompare, error) {
ctx := C.compare_init()
Expand Down Expand Up @@ -135,6 +144,9 @@ func (sc *SecureCompare) Begin() ([]byte, error) {
if !bool(C.compare_begin_size(sc.ctx, &outLen)) {
return nil, errors.New("Failed to get output size")
}
if sizeOverflow(outLen) {
return nil, ErrOverflow
}

out := make([]byte, outLen)

Expand All @@ -157,6 +169,9 @@ func (sc *SecureCompare) Proceed(data []byte) ([]byte, error) {
if !bool(C.compare_proceed_size(sc.ctx, unsafe.Pointer(&data[0]), C.size_t(len(data)), &outLen)) {
return nil, errors.New("Failed to get output size")
}
if sizeOverflow(outLen) {
return nil, ErrOverflow
}

if 0 == outLen {
outLen++
Expand Down
14 changes: 13 additions & 1 deletion gothemis/keys/keypair.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,9 @@ static bool gen_keys(int key_type, void *private, size_t priv_len, void *public,
import "C"

import (
"github.com/cossacklabs/themis/gothemis/errors"
"unsafe"

"github.com/cossacklabs/themis/gothemis/errors"
)

// Type of Themis key.
Expand All @@ -75,6 +76,7 @@ const (
// Errors returned by key generation.
var (
ErrInvalidType = errors.NewWithCode(errors.InvalidParameter, "invalid key type specified")
ErrOverflow = errors.NewWithCode(errors.NoMemory, "key generator cannot allocate enough memory")
)

// PrivateKey stores a ECDSA or RSA private key.
Expand Down Expand Up @@ -103,6 +105,9 @@ func New(keytype int) (*Keypair, error) {
if !bool(C.get_key_size(C.int(keytype), &privLen, &pubLen)) {
return nil, errors.New("Failed to get needed key sizes")
}
if sizeOverflow(privLen) || sizeOverflow(pubLen) {
return nil, ErrOverflow
}

priv := make([]byte, int(privLen), int(privLen))
pub := make([]byte, int(pubLen), int(pubLen))
Expand All @@ -116,3 +121,10 @@ func New(keytype int) (*Keypair, error) {
Public: &PublicKey{Value: pub},
}, nil
}

// C returns sizes as size_t but Go expresses buffer lengths as int.
// Make sure that all sizes are representable in Go and there is no overflows.
func sizeOverflow(n C.size_t) bool {
const maxInt = int(^uint(0) >> 1)
return n > C.size_t(maxInt)
}
3 changes: 3 additions & 0 deletions gothemis/keys/symmetric.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ func NewSymmetricKey() (*SymmetricKey, error) {
if !bool(C.get_sym_key_size(&len)) {
return nil, errors.New("Failed to get symmetric key size")
}
if sizeOverflow(len) {
return nil, ErrOverflow
}

key := make([]byte, int(len), int(len))
if !bool(C.gen_sym_key(unsafe.Pointer(&key[0]), len)) {
Expand Down
14 changes: 13 additions & 1 deletion gothemis/message/message.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,10 @@ static bool process(const void *priv, size_t priv_len, const void *public, size_
*/
import "C"
import (
"unsafe"

"github.com/cossacklabs/themis/gothemis/errors"
"github.com/cossacklabs/themis/gothemis/keys"
"unsafe"
)

const (
Expand All @@ -80,6 +81,7 @@ var (
ErrMissingMessage = errors.NewWithCode(errors.InvalidParameter, "empty message for Secure Cell")
ErrMissingPublicKey = errors.NewWithCode(errors.InvalidParameter, "empty peer public key for Secure Message")
ErrMissingPrivateKey = errors.NewWithCode(errors.InvalidParameter, "empty private key for Secure Message")
ErrOverflow = errors.NewWithCode(errors.NoMemory, "Secure Message cannot allocate enough memory")
)

// SecureMessage provides a sequence-independent, stateless, contextless messaging system.
Expand All @@ -95,6 +97,13 @@ func New(private *keys.PrivateKey, peerPublic *keys.PublicKey) *SecureMessage {
return &SecureMessage{private, peerPublic}
}

// C returns sizes as size_t but Go expresses buffer lengths as int.
// Make sure that all sizes are representable in Go and there is no overflows.
func sizeOverflow(n C.size_t) bool {
const maxInt = int(^uint(0) >> 1)
return n > C.size_t(maxInt)
}

func messageProcess(private *keys.PrivateKey, peerPublic *keys.PublicKey, message []byte, mode int) ([]byte, error) {
if nil == message || 0 == len(message) {
return nil, ErrMissingMessage
Expand Down Expand Up @@ -128,6 +137,9 @@ func messageProcess(private *keys.PrivateKey, peerPublic *keys.PublicKey, messag
&outputLength)) {
return nil, errors.New("Failed to get output size")
}
if sizeOverflow(outputLength) {
return nil, ErrOverflow
}

output := make([]byte, int(outputLength), int(outputLength))
if !bool(C.process(priv,
Expand Down
Loading

0 comments on commit 66b6727

Please sign in to comment.