Skip to content
This repository has been archived by the owner on Apr 23, 2024. It is now read-only.

Address issues in smart contract from security audit #93

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
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
16 changes: 10 additions & 6 deletions app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,9 @@ type Config struct {
StellarBridgeAccount string `toml:"stellar_bridge_account" valid:"stellar_accountid"`
StellarPrivateKey string `toml:"stellar_private_key" valid:"stellar_seed"`

EthereumRPCURL string `toml:"ethereum_rpc_url" valid:"-"`
EthereumBridgeAddress string `toml:"ethereum_bridge_address" valid:"-"`
EthereumBridgeConfigVersion uint32 `toml:"ethereum_bridge_config_version" valid:"-"`
EthereumPrivateKey string `toml:"ethereum_private_key" valid:"-"`
EthereumRPCURL string `toml:"ethereum_rpc_url" valid:"-"`
EthereumBridgeAddress string `toml:"ethereum_bridge_address" valid:"-"`
EthereumPrivateKey string `toml:"ethereum_private_key" valid:"-"`

AssetMapping []backend.AssetMappingConfigEntry `toml:"asset_mapping" valid:"-"`

Expand Down Expand Up @@ -147,10 +146,15 @@ func (a *App) initWorker(config Config, client *horizonclient.Client, ethObserve

converter, err := backend.NewAssetConverter(config.AssetMapping)
if err != nil {
log.Fatal("unable to create asset converter", err)
log.Fatalf("unable to create asset converter: %v", err)
}

domainSeparator, err := ethObserver.GetDomainSeparator(a.appCtx)
if err != nil {
log.Fatalf("unable to fetch domain separator: %v", err)
}

ethSigner, err := ethereum.NewSigner(config.EthereumPrivateKey, config.EthereumBridgeConfigVersion)
ethSigner, err := ethereum.NewSigner(config.EthereumPrivateKey, domainSeparator)
if err != nil {
log.Fatalf("cannot create ethereum signer: %v", err)
}
Expand Down
21 changes: 10 additions & 11 deletions app/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,16 @@ func TestParseConfig(t *testing.T) {
require.NoError(t, err)

expected := Config{
Port: 8000,
AdminPort: 6060,
PostgresDSN: "dbname=starbridge user=starbridge",
HorizonURL: "https://horizon-testnet.stellar.org",
NetworkPassphrase: "Test SDF Network ; September 2015",
StellarBridgeAccount: "GAJKCRY6CIOXRIVK55ALOOJA327XN4JZ5KKN7YCTT3WM5W6BMFXMVQC2",
StellarPrivateKey: "SCSTO3PMPM2BNLR2MYKVHWCJ2FNHQGFWKPOFH6UX4N3HO6HMK4JBSJ6F",
EthereumRPCURL: "https://ethereum-goerli-rpc.allthatnode.com",
EthereumBridgeAddress: "0xD0675839A6C2c3412a3026Aa5F521Ea1e948E526",
EthereumBridgeConfigVersion: 0,
EthereumPrivateKey: "2aecee1800342bae06228ed990a152563b8dedf5fe15e3eab4b44854c9e001e5",
Port: 8000,
AdminPort: 6060,
PostgresDSN: "dbname=starbridge user=starbridge",
HorizonURL: "https://horizon-testnet.stellar.org",
NetworkPassphrase: "Test SDF Network ; September 2015",
StellarBridgeAccount: "GAJKCRY6CIOXRIVK55ALOOJA327XN4JZ5KKN7YCTT3WM5W6BMFXMVQC2",
StellarPrivateKey: "SCSTO3PMPM2BNLR2MYKVHWCJ2FNHQGFWKPOFH6UX4N3HO6HMK4JBSJ6F",
EthereumRPCURL: "https://ethereum-goerli-rpc.allthatnode.com",
EthereumBridgeAddress: "0xD0675839A6C2c3412a3026Aa5F521Ea1e948E526",
EthereumPrivateKey: "2aecee1800342bae06228ed990a152563b8dedf5fe15e3eab4b44854c9e001e5",
AssetMapping: []backend.AssetMappingConfigEntry{
{
StellarAsset: "ETH:GAJKCRY6CIOXRIVK55ALOOJA327XN4JZ5KKN7YCTT3WM5W6BMFXMVQC2",
Expand Down
1 change: 0 additions & 1 deletion app/testdata/example.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ stellar_bridge_account="GAJKCRY6CIOXRIVK55ALOOJA327XN4JZ5KKN7YCTT3WM5W6BMFXMVQC2
stellar_private_key="SCSTO3PMPM2BNLR2MYKVHWCJ2FNHQGFWKPOFH6UX4N3HO6HMK4JBSJ6F"
ethereum_rpc_url="https://ethereum-goerli-rpc.allthatnode.com"
ethereum_bridge_address="0xD0675839A6C2c3412a3026Aa5F521Ea1e948E526"
ethereum_bridge_config_version=0
ethereum_private_key="2aecee1800342bae06228ed990a152563b8dedf5fe15e3eab4b44854c9e001e5"

[[asset_mapping]]
Expand Down
6 changes: 6 additions & 0 deletions ethereum/observer.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,3 +212,9 @@ func (o Observer) GetRequestStatus(ctx context.Context, requestID common.Hash) (
BlockNumber: blockNumber.Uint64(),
}, nil
}

// GetDomainSeparator calls the domainSeparator public attribute on the bridge contract
// and returns its value
func (o Observer) GetDomainSeparator(ctx context.Context) ([32]byte, error) {
return o.caller.DomainSeparator(&bind.CallOpts{Context: ctx})
}
23 changes: 11 additions & 12 deletions ethereum/signer.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
)

var (
uint256 = mustType("uint256")
bytes32 = mustType("bytes32")
withdrawERC20Type = mustTupleType([]abi.ArgumentMarshaling{
{Name: "id", Type: "bytes32"},
Expand Down Expand Up @@ -50,21 +49,21 @@ func mustTupleType(components []abi.ArgumentMarshaling) abi.Type {
// Signer represents an ethereum validator account which is
// authorized to approve withdrawals from the bridge smart contract.
type Signer struct {
privateKey *ecdsa.PrivateKey
version *big.Int
address common.Address
privateKey *ecdsa.PrivateKey
domainSeparator [32]byte
address common.Address
}

// NewSigner constructs a new Signer instance
func NewSigner(privateKey string, bridgeConfigVersion uint32) (Signer, error) {
func NewSigner(privateKey string, domainSeparator [32]byte) (Signer, error) {
parsed, err := crypto.HexToECDSA(privateKey)
if err != nil {
return Signer{}, err
}
return Signer{
privateKey: parsed,
version: big.NewInt(int64(bridgeConfigVersion)),
address: crypto.PubkeyToAddress(parsed.PublicKey),
privateKey: parsed,
domainSeparator: domainSeparator,
address: crypto.PubkeyToAddress(parsed.PublicKey),
}, nil
}

Expand Down Expand Up @@ -102,13 +101,13 @@ func (s Signer) SignWithdrawal(

func (s Signer) signWithdrawERC20Request(request solidity.WithdrawERC20Request) ([]byte, error) {
arguments := abi.Arguments{
{Type: uint256},
{Type: bytes32},
{Type: bytes32},
{Type: withdrawERC20Type},
}

abiEncoded, err := arguments.Pack(
s.version,
s.domainSeparator,
crypto.Keccak256Hash([]byte("withdrawERC20")),
request,
)
Expand All @@ -120,13 +119,13 @@ func (s Signer) signWithdrawERC20Request(request solidity.WithdrawERC20Request)

func (s Signer) signWithdrawETHRequest(request solidity.WithdrawETHRequest) ([]byte, error) {
arguments := abi.Arguments{
{Type: uint256},
{Type: bytes32},
{Type: bytes32},
{Type: withdrawETHType},
}

abiEncoded, err := arguments.Pack(
s.version,
s.domainSeparator,
crypto.Keccak256Hash([]byte("withdrawETH")),
request,
)
Expand Down
6 changes: 3 additions & 3 deletions ethereum/signer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
func createSigner(t *testing.T) Signer {
signer, err := NewSigner(
"51138e68e8a5fa906d38c5b42bc01b805d7adb3fce037743fb406bb10aa83307",
0,
[32]byte{1, 2, 3},
)
assert.NoError(t, err)
return signer
Expand Down Expand Up @@ -44,14 +44,14 @@ func TestSigner_SignWithdrawal(t *testing.T) {
recipient: common.HexToAddress("0x123"),
token: common.HexToAddress("0x456"),
amount: big.NewInt(200),
expected: "2a4fead286732e459cc4f167aa34f6ca6b83fa4e7c993429582a048020a4c2840f47a9903257266605de9975d2122d1db8697b7398474889aafaa3c9d1b4bd6c1c",
expected: "d668c6d190f0a1dcb03b5540794479659a5d46cfa741d2e52f65b9d5e4afae420dae19570a2dd871486c886c0e19511eb1bb39299baa99baf7b73ef30190e0d91c",
},
{
id: common.HexToHash("0x55"),
expiration: 200,
recipient: common.HexToAddress("0x321"),
amount: big.NewInt(100),
expected: "40cd596f0d1683bbe42c6fc57220ecfda15d78d5ce9ccdf68c4da4d9a4d1a6bf63d249d59b17ead7c59b4b4da9755aad6eb9eefe656685322de074128370d31e1c",
expected: "2f7c8c16b1cec4063b9df791ead00e4db539072963b07bec12957c8680dc1eee6d41b2e906746b9b619af2ffb5792d794c81a90381d8b26e40b913e5a0fe0f821b",
},
} {
signature, err := signer.SignWithdrawal(
Expand Down
24 changes: 12 additions & 12 deletions integration/integration.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ func NewIntegrationTest(t *testing.T, config Config) *Test {
}

test.runComposeCommand("down", "-v")
test.runComposeCommand("build")
test.runComposeCommand("up", "--detach", "--quiet-pull", "--no-color", "starbridge-postgres")
test.runComposeCommand("up", "--detach", "--quiet-pull", "--no-color", "quickstart")
test.runComposeCommand("up", "--detach", "--no-color", "ethereum-node")
Expand Down Expand Up @@ -251,18 +252,17 @@ func (i *Test) StartStarbridge(id int, config Config, ingestSequence uint32) err
i.signerKeys[id] = keypair.MustRandom()

i.app[id] = app.NewApp(app.Config{
Port: 9000 + uint16(id),
PostgresDSN: fmt.Sprintf("postgres://postgres:mysecretpassword@%s:5641/starbridge%d?sslmode=disable", dockerHost, id),
HorizonURL: fmt.Sprintf("http://%s:8000/", dockerHost),
NetworkPassphrase: StandaloneNetworkPassphrase,
StellarBridgeAccount: i.mainAccount.GetAccountID(),
StellarPrivateKey: i.signerKeys[id].Seed(),
EthereumRPCURL: EthereumRPCURL,
EthereumBridgeAddress: EthereumBridgeAddress,
EthereumBridgeConfigVersion: 0,
EthereumPrivateKey: ethPrivateKeys[id],
EthereumFinalityBuffer: 0,
WithdrawalWindow: config.WithdrawalWindow,
Port: 9000 + uint16(id),
PostgresDSN: fmt.Sprintf("postgres://postgres:mysecretpassword@%s:5641/starbridge%d?sslmode=disable", dockerHost, id),
HorizonURL: fmt.Sprintf("http://%s:8000/", dockerHost),
NetworkPassphrase: StandaloneNetworkPassphrase,
StellarBridgeAccount: i.mainAccount.GetAccountID(),
StellarPrivateKey: i.signerKeys[id].Seed(),
EthereumRPCURL: EthereumRPCURL,
EthereumBridgeAddress: EthereumBridgeAddress,
EthereumPrivateKey: ethPrivateKeys[id],
EthereumFinalityBuffer: 0,
WithdrawalWindow: config.WithdrawalWindow,
AssetMapping: []backend.AssetMappingConfigEntry{
{
StellarAsset: "native",
Expand Down
Loading