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

fix(dot/subscription): unsafe type casting from untrusted input #2529

Merged
merged 14 commits into from
May 30, 2022
Merged
Show file tree
Hide file tree
Changes from 7 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
70 changes: 47 additions & 23 deletions dot/rpc/subscription/websocket.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,13 @@ type httpclient interface {
Do(*http.Request) (*http.Response, error)
}

var errCannotReadFromWebsocket = errors.New("cannot read message from websocket")
var errCannotUnmarshalMessage = errors.New("cannot unmarshal webasocket message data")
var (
errUnexpectedType = errors.New("unexpected type")
errUnexpectedParamLen = errors.New("unexpected params length")
errCannotReadFromWebsocket = errors.New("cannot read message from websocket")
errCannotUnmarshalMessage = errors.New("cannot unmarshal websocket message data")
)

var logger = log.NewFromGlobal(log.AddContext("pkg", "rpc/subscription"))

// WSConn struct to hold WebSocket Connection references
Expand Down Expand Up @@ -159,25 +164,26 @@ func (c *WSConn) initStorageChangeListener(reqID float64, params interface{}) (L
wsconn: c,
}

pA, ok := params.([]interface{})
if !ok {
return nil, fmt.Errorf("unknown parameter type")
}
for _, param := range pA {
switch p := param.(type) {
case []interface{}:
for _, pp := range param.([]interface{}) {
data, ok := pp.(string)
if !ok {
return nil, fmt.Errorf("unknown parameter type")
}
stgobs.filter[data] = []byte{}
switch filter := params.(type) {
case string:
stgobs.filter[filter] = []byte{}
EclesioMeloJunior marked this conversation as resolved.
Show resolved Hide resolved
case []string:
for _, key := range filter {
stgobs.filter[key] = []byte{}
}
// the bellow case is needed to cover a interface{} slice full of strings
// as `[]interface{"a", "b"}` is not the same as `[]string{"a", "b"}`
case []interface{}:
EclesioMeloJunior marked this conversation as resolved.
Show resolved Hide resolved
EclesioMeloJunior marked this conversation as resolved.
Show resolved Hide resolved
for _, interfaceKey := range filter {
key, ok := interfaceKey.(string)
if !ok {
return nil, fmt.Errorf("%w: %T, expected type string", errUnexpectedType, interfaceKey)
}
case string:
stgobs.filter[p] = []byte{}
default:
return nil, fmt.Errorf("unknown parameter type")

stgobs.filter[key] = []byte{}
}
default:
return nil, fmt.Errorf("%w: %T, expected type string, []string or []interface{}", errUnexpectedType, params)
}

c.mu.Lock()
Expand Down Expand Up @@ -265,14 +271,32 @@ func (c *WSConn) initAllBlocksListerner(reqID float64, _ interface{}) (Listener,
}

func (c *WSConn) initExtrinsicWatch(reqID float64, params interface{}) (Listener, error) {
qdm12 marked this conversation as resolved.
Show resolved Hide resolved
pA := params.([]interface{})
var encodedExtrinsic string

if len(pA) != 1 {
return nil, errors.New("expecting only one parameter")
switch encodedHex := params.(type) {
case []string:
if len(encodedHex) != 1 {
return nil, fmt.Errorf("%w: %d, expected 1 param", errUnexpectedParamLen, len(encodedHex))
EclesioMeloJunior marked this conversation as resolved.
Show resolved Hide resolved
}
encodedExtrinsic = encodedHex[0]
// the bellow case is needed to cover a interface{} slice containing one string
// as `[]interface{"a"}` is not the same as `[]string{"a"}`
case []interface{}:
EclesioMeloJunior marked this conversation as resolved.
Show resolved Hide resolved
if len(encodedHex) != 1 {
return nil, fmt.Errorf("%w: %d, expected 1 param", errUnexpectedParamLen, len(encodedHex))
}

var ok bool
encodedExtrinsic, ok = encodedHex[0].(string)
if !ok {
return nil, fmt.Errorf("%w: %T, expected type string", errUnexpectedType, encodedHex[0])
}
default:
return nil, fmt.Errorf("%w: %T, expected type []string or []interface{}", errUnexpectedType, params)
}

// The passed parameter should be a HEX of a SCALE encoded extrinsic
extBytes, err := common.HexToBytes(pA[0].(string))
extBytes, err := common.HexToBytes(encodedExtrinsic)
if err != nil {
return nil, err
}
Expand Down
22 changes: 12 additions & 10 deletions dot/rpc/subscription/websocket_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ func TestWSConn_HandleComm(t *testing.T) {
res, err = wsconn.initStorageChangeListener(1, nil)
require.Nil(t, res)
require.Len(t, wsconn.Subscriptions, 0)
require.EqualError(t, err, "unknown parameter type")
require.ErrorIs(t, err, errUnexpectedType)
require.EqualError(t, err, "unexpected type: <nil>, expected type string, []string or []interface{}")

res, err = wsconn.initStorageChangeListener(2, []interface{}{})
require.NotNil(t, res)
Expand All @@ -55,33 +56,34 @@ func TestWSConn_HandleComm(t *testing.T) {
require.NoError(t, err)
require.Equal(t, []byte(`{"jsonrpc":"2.0","result":1,"id":2}`+"\n"), msg)

res, err = wsconn.initStorageChangeListener(3, []interface{}{"0x26aa"})
res, err = wsconn.initStorageChangeListener(3, "0x26aa")
require.NotNil(t, res)
require.NoError(t, err)
require.Len(t, wsconn.Subscriptions, 2)
_, msg, err = c.ReadMessage()
require.NoError(t, err)
require.Equal(t, []byte(`{"jsonrpc":"2.0","result":2,"id":3}`+"\n"), msg)

var testFilters = []interface{}{}
var testFilter1 = []interface{}{"0x26aa", "0x26a1"}
res, err = wsconn.initStorageChangeListener(4, append(testFilters, testFilter1))
require.NotNil(t, res)
var testFilter1 = []string{"0x26aa", "0x26a1"}
res, err = wsconn.initStorageChangeListener(4, testFilter1)
require.NoError(t, err)
require.NotNil(t, res)
require.Len(t, wsconn.Subscriptions, 3)
_, msg, err = c.ReadMessage()
require.NoError(t, err)
require.Equal(t, []byte(`{"jsonrpc":"2.0","result":3,"id":4}`+"\n"), msg)

var testFilterWrongType = []interface{}{"0x26aa", 1}
res, err = wsconn.initStorageChangeListener(5, append(testFilters, testFilterWrongType))
require.EqualError(t, err, "unknown parameter type")
res, err = wsconn.initStorageChangeListener(5, testFilterWrongType)
require.ErrorIs(t, err, errUnexpectedType)
require.EqualError(t, err, "unexpected type: int, expected type string")
require.Nil(t, res)
// keep subscriptions len == 3, no additions was made
require.Len(t, wsconn.Subscriptions, 3)

res, err = wsconn.initStorageChangeListener(6, []interface{}{1})
require.EqualError(t, err, "unknown parameter type")
require.ErrorIs(t, err, errUnexpectedType)
require.EqualError(t, err, "unexpected type: int, expected type string")
require.Nil(t, res)
require.Len(t, wsconn.Subscriptions, 3)

Expand Down Expand Up @@ -207,7 +209,7 @@ func TestWSConn_HandleComm(t *testing.T) {
wsconn.CoreAPI = modules.NewMockCoreAPI()
wsconn.BlockAPI = nil
wsconn.TxStateAPI = modules.NewMockTransactionStateAPI()
listner, err := wsconn.initExtrinsicWatch(0, []interface{}{"NotHex"})
listner, err := wsconn.initExtrinsicWatch(0, []string{"NotHex"})
EclesioMeloJunior marked this conversation as resolved.
Show resolved Hide resolved
require.EqualError(t, err, "could not byteify non 0x prefixed string: NotHex")
require.Nil(t, listner)

Expand Down