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

Avoid overflows on 32-bit systems #677

Merged
merged 7 commits into from
Jul 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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