Skip to content

Commit

Permalink
Fix SherClockHolmes#27: Panic on large notification payloads
Browse files Browse the repository at this point in the history
This commit fixes a case where, with large notification payloads, pad
would call make with a negative length, triggering a panic.
  • Loading branch information
abustany committed Oct 9, 2019
1 parent 7ed4780 commit b9d513c
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 2 deletions.
12 changes: 10 additions & 2 deletions webpush.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,9 @@ func SendNotification(message []byte, s *Subscription, options *Options) (*http.
// Pad content to max record size - 16 - header
// Padding ending delimeter
dataBuf.Write([]byte("\x02"))
pad(dataBuf, recordLength-recordBuf.Len())
if err := pad(dataBuf, recordLength-recordBuf.Len()); err != nil {
return nil, err
}

// Compose the ciphertext
ciphertext := gcm.Seal([]byte{}, nonce, dataBuf.Bytes(), nil)
Expand Down Expand Up @@ -243,10 +245,16 @@ func getHKDFKey(hkdf io.Reader, length int) ([]byte, error) {
return key, nil
}

func pad(payload *bytes.Buffer, maxPadLen int) {
func pad(payload *bytes.Buffer, maxPadLen int) error {
payloadLen := payload.Len()
if payloadLen > maxPadLen {
return errors.New("payload is too large")
}

padLen := maxPadLen - payloadLen

padding := make([]byte, padLen)
payload.Write(padding)

return nil
}
15 changes: 15 additions & 0 deletions webpush_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package webpush

import (
"net/http"
"strings"
"testing"
)

Expand Down Expand Up @@ -76,3 +77,17 @@ func TestSendNotificationToStandardEncodedSubscription(t *testing.T) {
)
}
}

func TestSendTooLargeNotification(t *testing.T) {
_, err := SendNotification([]byte(strings.Repeat("Test", int(MaxRecordSize))), getStandardEncodedTestSubscription(), &Options{
HTTPClient: &testHTTPClient{},
Subscriber: "<[email protected]>",
Topic: "test_topic",
TTL: 0,
Urgency: "low",
VAPIDPrivateKey: "testKey",
})
if err == nil {
t.Fatal("Expected error with too large payload")
}
}

0 comments on commit b9d513c

Please sign in to comment.