xmtp / xmtp-node-go

Software for the nodes that currently form the XMTP network
MIT License
10 stars 3 forks source link

Server panics when passed base64 encoded invalid token #225

Closed nakajima closed 1 year ago

nakajima commented 1 year ago

If the server receives authorization: Bearer aGk= metadata, there's a panic somewhere around here.

View crash output
xmtp-node  | md is map[:authority:[localhost] authorization:[Bearer aGk=] content-type:[application/grpc] user-agent:[grpc-swift-nio/1.13.0] x-app-version:[0.0.0-development] x-client-version:[0.0.0-development]]
xmtp-node  | panic: runtime error: invalid memory address or nil pointer dereference
xmtp-node  | [signal SIGSEGV: segmentation violation code=0x1 addr=0x28 pc=0x1013270]
xmtp-node  | 
xmtp-node  | goroutine 196 [running]:
xmtp-node  | github.com/xmtp/xmtp-node-go/pkg/api.createIdentitySignRequest(0x0)
xmtp-node  |    /app/pkg/api/authentication.go:64 +0x30
xmtp-node  | github.com/xmtp/xmtp-node-go/pkg/api.recoverWalletAddress({0x4002753718?, 0x470db4?}, 0x0)
xmtp-node  |    /app/pkg/api/authentication.go:75 +0x28
xmtp-node  | github.com/xmtp/xmtp-node-go/pkg/api.validateToken({0x18bf828, 0x4002769980}, 0x18ae510?, 0x400070d540, {0x0?, 0x1?, 0x24e0b00?})
xmtp-node  |    /app/pkg/api/authentication.go:34 +0x44
xmtp-node  | github.com/xmtp/xmtp-node-go/pkg/api.(*WalletAuthorizer).authorize(0x40009da4c0, {0x18bf828, 0x4002769980}, {0x1328260?, 0x4000713100})
xmtp-node  |    /app/pkg/api/interceptor.go:101 +0x208
xmtp-node  | github.com/xmtp/xmtp-node-go/pkg/api.(*WalletAuthorizer).Unary.func1({0x18bf828, 0x4002769980}, {0x1328260?, 0x4000713100}, 0x40000e8000?, 0x40001210c8)
xmtp-node  |    /app/pkg/api/interceptor.go:51 +0x70
xmtp-node  | github.com/grpc-ecosystem/go-grpc-middleware.ChainUnaryServer.func1.1.1({0x18bf828?, 0x4002769980?}, {0x1328260?, 0x4000713100?})
xmtp-node  |    /go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware@v1.3.0/chain.go:25 +0x40
xmtp-node  | github.com/xmtp/xmtp-node-go/pkg/api.(*TelemetryInterceptor).Unary.func1({0x18bf828, 0x4002769980}, {0x1328260?, 0x4000713100?}, 0x40001118e0, 0x400070d4f0?)
xmtp-node  |    /app/pkg/api/telemetry.go:37 +0x44
xmtp-node  | github.com/grpc-ecosystem/go-grpc-middleware.ChainUnaryServer.func1.1.1({0x18bf828?, 0x4002769980?}, {0x1328260?, 0x4000713100?})
xmtp-node  |    /go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware@v1.3.0/chain.go:25 +0x40
xmtp-node  | github.com/grpc-ecosystem/go-grpc-prometheus.(*ServerMetrics).UnaryServerInterceptor.func1({0x18bf828, 0x4002769980}, {0x1328260, 0x4000713100}, 0x3740?, 0x4000111920)
xmtp-node  |    /go/pkg/mod/github.com/grpc-ecosystem/go-grpc-prometheus@v1.2.0/server_metrics.go:107 +0x78
xmtp-node  | github.com/grpc-ecosystem/go-grpc-middleware.ChainUnaryServer.func1.1.1({0x18bf828?, 0x4002769980?}, {0x1328260?, 0x4000713100?})
xmtp-node  |    /go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware@v1.3.0/chain.go:25 +0x40
xmtp-node  | github.com/grpc-ecosystem/go-grpc-middleware.ChainUnaryServer.func1({0x18bf828, 0x4002769980}, {0x1328260, 0x4000713100}, 0x40013beaa8?, 0xa2867c?)
xmtp-node  |    /go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware@v1.3.0/chain.go:34 +0xbc
xmtp-node  | github.com/xmtp/proto/v3/go/message_api/v1._MessageApi_Publish_Handler({0x1367180?, 0x400003e820}, {0x18bf828, 0x4002769980}, 0x40000a9dc0, 0x4000112ba0)
xmtp-node  |    /go/pkg/mod/github.com/xmtp/proto/v3@v3.12.0/go/message_api/v1/message_api_grpc.pb.go:200 +0x138
xmtp-node  | google.golang.org/grpc.(*Server).processUnaryRPC(0x400013a1e0, {0x18c89c0, 0x4000b3b6c0}, 0x4000000360, 0x4000112f30, 0x21ff980, 0x0)
xmtp-node  |    /go/pkg/mod/google.golang.org/grpc@v1.48.0/server.go:1295 +0x9d4
xmtp-node  | google.golang.org/grpc.(*Server).handleStream(0x400013a1e0, {0x18c89c0, 0x4000b3b6c0}, 0x4000000360, 0x0)
xmtp-node  |    /go/pkg/mod/google.golang.org/grpc@v1.48.0/server.go:1636 +0x854
xmtp-node  | google.golang.org/grpc.(*Server).serveStreams.func1.2()
xmtp-node  |    /go/pkg/mod/google.golang.org/grpc@v1.48.0/server.go:932 +0x88
xmtp-node  | created by google.golang.org/grpc.(*Server).serveStreams.func1
xmtp-node  |    /go/pkg/mod/google.golang.org/grpc@v1.48.0/server.go:930 +0x29c
michaelx11 commented 1 year ago

Confirmed this just now. Great find!

mkobetic commented 1 year ago

Looks like decodeToken can return an empty token?

michaelx11 commented 1 year ago

I think it can deserialize a token correctly but with a nil PubKey pointer?

From stacktrace we see a nil pointer. Below is excerpts from pkg/api/authentication.go + stacktrace:

func validateToken(ctx context.Context, log *zap.Logger, token *messagev1.Token, now time.Time) (wallet types.WalletAddr, err error) {
    // Validate IdentityKey signature.
    pubKey := token.IdentityKey
    recoveredWalletAddress, err := recoverWalletAddress(ctx, pubKey)
...
}

func recoverWalletAddress(ctx context.Context, identityKey *envelope.PublicKey) (wallet types.WalletAddr, err error) {
...
}

from stacktrace:
github.com/xmtp/xmtp-node-go/pkg/api.recoverWalletAddress({0x4002753718?, 0x470db4?}, 0x0) <- nil pointer for *envelope.PubKey
michaelx11 commented 1 year ago

I'm working on a patch now. I'm going to add checks on the decodeToken side and within recoverWalletAddress.

mkobetic commented 1 year ago

I guess it depends on definition of "correctly" just because proto.Unmarshal doesn't given an error, we don't have to treat it as everything is fine.