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

swarm/pss: Reduce input vulnerabilities #18304

Merged
merged 5 commits into from
Dec 18, 2018
Merged
Show file tree
Hide file tree
Changes from 2 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: 16 additions & 0 deletions swarm/pss/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,14 +157,23 @@ func (pssapi *API) StringToTopic(topicstring string) (Topic, error) {
}

func (pssapi *API) SendAsym(pubkeyhex string, topic Topic, msg hexutil.Bytes) error {
if !checkMsg(msg) {
nolash marked this conversation as resolved.
Show resolved Hide resolved
return errors.New("invalid message")
nolash marked this conversation as resolved.
Show resolved Hide resolved
}
return pssapi.Pss.SendAsym(pubkeyhex, topic, msg[:])
}

func (pssapi *API) SendSym(symkeyhex string, topic Topic, msg hexutil.Bytes) error {
if !checkMsg(msg) {
return errors.New("invalid message")
}
return pssapi.Pss.SendSym(symkeyhex, topic, msg[:])
}

func (pssapi *API) SendRaw(addr hexutil.Bytes, topic Topic, msg hexutil.Bytes) error {
if !checkMsg(msg) {
return errors.New("invalid message")
}
return pssapi.Pss.SendRaw(PssAddress(addr), topic, msg[:])
}

Expand All @@ -177,3 +186,10 @@ func (pssapi *API) GetPeerTopics(pubkeyhex string) ([]Topic, error) {
func (pssapi *API) GetPeerAddress(pubkeyhex string, topic Topic) (PssAddress, error) {
return pssapi.Pss.getPeerAddress(pubkeyhex, topic)
}

func checkMsg(msg []byte) bool {
nolash marked this conversation as resolved.
Show resolved Hide resolved
if msg == nil || len(msg) == 0 {
nolash marked this conversation as resolved.
Show resolved Hide resolved
return false
}
return true
}
30 changes: 25 additions & 5 deletions swarm/pss/pss.go
Original file line number Diff line number Diff line change
Expand Up @@ -396,9 +396,11 @@ func (p *Pss) handlePssMsg(ctx context.Context, msg interface{}) error {
// raw is simplest handler contingency to check, so check that first
var isRaw bool
if pssmsg.isRaw() {
if !p.topicHandlerCaps[psstopic].raw {
log.Debug("No handler for raw message", "topic", psstopic)
return nil
if _, ok := p.topicHandlerCaps[psstopic]; ok {
if !p.topicHandlerCaps[psstopic].raw {
log.Debug("No handler for raw message", "topic", psstopic)
return nil
}
}
isRaw = true
}
Expand Down Expand Up @@ -529,6 +531,9 @@ func (p *Pss) isSelfPossibleRecipient(msg *PssMsg, prox bool) bool {
// The value in `address` will be used as a routing hint for the
// public key / topic association
func (p *Pss) SetPeerPublicKey(pubkey *ecdsa.PublicKey, topic Topic, address *PssAddress) error {
if !checkAddress(address) {
return errors.New("invalid address")
nolash marked this conversation as resolved.
Show resolved Hide resolved
}
pubkeybytes := crypto.FromECDSAPub(pubkey)
if len(pubkeybytes) == 0 {
return fmt.Errorf("invalid public key: %v", pubkey)
Expand Down Expand Up @@ -570,6 +575,9 @@ func (p *Pss) GenerateSymmetricKey(topic Topic, address *PssAddress, addToCache
// Returns a string id that can be used to retrieve the key bytes
// from the whisper backend (see pss.GetSymmetricKey())
func (p *Pss) SetSymmetricKey(key []byte, topic Topic, address *PssAddress, addtocache bool) (string, error) {
if !checkAddress(address) {
return "", errors.New("invalid address")
}
return p.setSymmetricKey(key, topic, address, addtocache, true)
}

Expand Down Expand Up @@ -751,6 +759,9 @@ func (p *Pss) enqueue(msg *PssMsg) error {
//
// Will fail if raw messages are disallowed
func (p *Pss) SendRaw(address PssAddress, topic Topic, msg []byte) error {
if !checkAddress(&address) {
return errors.New("invalid address")
nolash marked this conversation as resolved.
Show resolved Hide resolved
}
pssMsgParams := &msgParams{
raw: true,
}
Expand All @@ -770,8 +781,10 @@ func (p *Pss) SendRaw(address PssAddress, topic Topic, msg []byte) error {

// if we have a proxhandler on this topic
// also deliver message to ourselves
if p.isSelfPossibleRecipient(pssMsg, true) && p.topicHandlerCaps[topic].prox {
return p.process(pssMsg, true, true)
if _, ok := p.topicHandlerCaps[topic]; ok {
if p.isSelfPossibleRecipient(pssMsg, true) && p.topicHandlerCaps[topic].prox {
return p.process(pssMsg, true, true)
}
}
return nil
}
Expand Down Expand Up @@ -1034,3 +1047,10 @@ func (p *Pss) digestBytes(msg []byte) pssDigest {
copy(digest[:], key[:digestLength])
return digest
}

func checkAddress(addr *PssAddress) bool {
nolash marked this conversation as resolved.
Show resolved Hide resolved
if len(*addr) > addressLength {
nolash marked this conversation as resolved.
Show resolved Hide resolved
return false
}
return true
}
28 changes: 28 additions & 0 deletions swarm/pss/pss_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1008,6 +1008,34 @@ func TestRawAllow(t *testing.T) {
}
}

// BELOW HERE ARE TESTS USING THE SIMULATION FRAMEWORK

// tests that the API layer can handle edge case values
func TestApi(t *testing.T) {
clients, err := setupNetwork(2, true)
if err != nil {
t.Fatal(err)
}

topic := "0xdeadbeef"

err = clients[0].Call(nil, "pss_sendRaw", "0x", topic, "0x666f6f")
if err != nil {
t.Fatal(err)
}

err = clients[0].Call(nil, "pss_sendRaw", "0xabcdef", topic, "0x")
if err == nil {
t.Fatal("expected error on empty msg")
}

overflowAddr := [33]byte{}
err = clients[0].Call(nil, "pss_sendRaw", hexutil.Encode(overflowAddr[:]), topic, "0x666f6f")
if err == nil {
nolash marked this conversation as resolved.
Show resolved Hide resolved
t.Fatal("expected error on send too big address")
}
}

// verifies that nodes can send and receive raw (verbatim) messages
func TestSendRaw(t *testing.T) {
t.Run("32", testSendRaw)
Expand Down