Agoric / agoric-sdk

monorepo for the Agoric Javascript smart contract platform
Apache License 2.0
327 stars 208 forks source link

User should not able to bind a bound port #5787

Open simpletrontdip opened 2 years ago

simpletrontdip commented 2 years ago

From the code, looks like we're currently allowing a local address to be bound multiple times.

    if (boundPorts.has(localAddr)) {
      return boundPorts.get(localAddr);
    }

It could be very dangerous if someone can obtain an existing port, especially for ICA, because port-id is the only thing that the host-chain checks before making transactions, and our IBC module forwards anything it gets from the user.

Should we totally believe that networkVat would never be leaked? Or we would never mistakenly re-bind a port and pass it to an unwanted hand?

// XXX Should this raise an Error?
port = await E(networkVat).bind('/ibc-port/some-ica-bound-port')

// Revoke to close the existing authorized channel
await E(port).revoke()

// Reconnect to make an illegal one
connection = await E(port).connect('same-connection-string')

// Make transactions
E(connection).send({ /* some malicious transactions */})

From the original computer networking perspective, binding a bound port is prohibited. So if we have no special reasons for it, we may need to avoid that too

dckc commented 2 years ago

@arirubinstein would you please estimate the priority / severity of this? Maybe check with @warner ?