hyperledger / fabric-private-chaincode

FPC enables Confidential Chaincode Execution for Hyperledger Fabric using Intel SGX.
Apache License 2.0
161 stars 91 forks source link

max buffer length not checked on get_state #195

Closed bvavala closed 4 years ago

bvavala commented 4 years ago

When the enclave calls get_state, it passes the buffer, the buffer length and it expects the length of the written data as an output parameter. As the function calls go down the stack, they reach the get_state in ecc. Here, https://github.com/hyperledger-labs/fabric-private-chaincode/blob/9583608e8e50fa438b251b831f94e35ae2b77ab5/ecc/enclave/enclave_stub.go#L184 max_val_len is not checked and there may be a buffer overflow if the array is too short. Below there is a patch to avoid it.

diff --git a/ecc/enclave/enclave_stub.go b/ecc/enclave/enclave_stub.go
index 9334e5c..bfe542a 100644
--- a/ecc/enclave/enclave_stub.go
+++ b/ecc/enclave/enclave_stub.go
@@ -181,6 +181,10 @@ func get_state(key *C.char, val *C.uint8_t, max_val_len C.uint32_t, val_len *C.u
    if err != nil {
        panic("error while getting state")
    }
+   if C.uint32_t(len(data)) > max_val_len {
+       C._set_int(val_len, C.uint32_t(0))
+       return
+   }
    C._cpy_bytes(val, (*C.uint8_t)(C.CBytes(data)), C.uint32_t(len(data)))
    C._set_int(val_len, C.uint32_t(len(data)))
bvavala commented 4 years ago

Since today we are developing the Auction chaincode demo, it is worth emphasizing that a simple auction configuration already makes the getAuctionDetails fail.

g2flyer commented 4 years ago

fixed in R #196