search

kaspanet / kaspad

Kaspad was the reference full node Kaspa implementation written in Go (golang), now rewritten in Rust: https://github.com/kaspanet/rusty-kaspa
ISC License
457 stars 231 forks source link

Crashing kaspads with invalid kaspactl arguments - add testing for all RPC calls. #2035

Closed D-Stacks closed 2 years ago

D-Stacks commented 2 years ago

I noticed it is possible to crash kaspads by querying the kaspad with invalid arguments, in my example i queried ./kaspaclt GetBalanceByAddress <address> using an address that doesn't contain any prefix i.e. ('kaspa:', 'kaspatest'). I suspect the underlying problem is that execeptions, due to bad rpc input, are being raised in the kaspad, while processing the rpc call. These should be error handled properly, and sent back as an error over the rpc response. I suspect this might be the case for other calls also. I propose that Full testing for all rpc calls, could negate these kind of bugs in the future, and would ensure such bugs do not make it into kaspad in the future.

Below is the full stack trace:

 2022-04-22 11:59:16.229 [INF] TXMP: RPC Incoming connection from 127.0.0.1:40580
2022-04-22 11:59:16.234 [CRT] RPCS: Exiting: Fatal error in goroutine `routerInitializer-handleIncomingMessages 265`: Couldn't decode address 'qrddu0lv6zwxk50rajelqxgl5jr63d6uwnrv4cfqxaz2as7z09udg75f739nj': decoded address is of unknown format: invalid index of ':'
2022-04-22 11:59:16.234 [CRT] RPCS: Goroutine stack trace: goroutine 429 [running]:
runtime/debug.Stack()
    /usr/lib/go-1.18/src/runtime/debug/stack.go:24 +0x65
github.com/kaspanet/kaspad/util/panics.GoroutineWrapperFunc.func1({0xc2abd3, 0x28}, 0xc0220461e0)
    /home/user/Coding/kaspad/kaspad/util/panics/panics.go:30 +0x35
github.com/kaspanet/kaspad/app/rpc.(*Manager).routerInitializer(0xc01a5b4600, 0xc022046000, 0xc0220460f0)
    /home/user/Coding/kaspad/kaspad/app/rpc/rpc.go:65 +0x222
github.com/kaspanet/kaspad/infrastructure/network/netadapter.newNetConnection({0xd2db00?, 0xc014f58150}, 0xc01a5a3570, {0xc1449e?, 0x60?})
    /home/user/Coding/kaspad/kaspad/infrastructure/network/netadapter/netconnection.go:41 +0xf2
github.com/kaspanet/kaspad/infrastructure/network/netadapter.(*NetAdapter).onRPCConnectedHandler(0x68?, {0xd2db00?, 0xc014f58150?})
    /home/user/Coding/kaspad/kaspad/infrastructure/network/netadapter/netadapter.go:151 +0x3c
github.com/kaspanet/kaspad/infrastructure/network/netadapter/server/grpcserver.(*gRPCServer).handleInboundConnection(0xc00007f450, {0xd28ea8, 0xc0218d9fb0}, {0x7fc7a3531fb8?, 0xc018bc0ba0})
    /home/user/Coding/kaspad/kaspad/infrastructure/network/netadapter/server/grpcserver/grpc_server.go:113 +0x18b
github.com/kaspanet/kaspad/infrastructure/network/netadapter/server/grpcserver.(*rpcServer).MessageStream(0xc00007f450, {0xd2dc08, 0xc018bc0ba0})
    /home/user/Coding/kaspad/kaspad/infrastructure/network/netadapter/server/grpcserver/rpcserver.go:31 +0xbe
github.com/kaspanet/kaspad/infrastructure/network/netadapter/server/grpcserver/protowire._RPC_MessageStream_Handler({0xbb1a00?, 0xc00007f450}, {0xd2c6e8?, 0xc000432240})
    /home/user/Coding/kaspad/kaspad/infrastructure/network/netadapter/server/grpcserver/protowire/messages_grpc.pb.go:214 +0x9f
google.golang.org/grpc.(*Server).processStreamingRPC(0xc0004f2380, {0xd2df28, 0xc0001b3500}, 0xc0001ff560, 0xc0001b5d10, 0x11ac1e0, 0x0)
    /home/user/go/pkg/mod/google.golang.org/grpc@v1.38.0/server.go:1533 +0xd06
google.golang.org/grpc.(*Server).handleStream(0xc0004f2380, {0xd2df28, 0xc0001b3500}, 0xc0001ff560, 0x0)
    /home/user/go/pkg/mod/google.golang.org/grpc@v1.38.0/server.go:1613 +0x9d6
google.golang.org/grpc.(*Server).serveStreams.func1.2()
    /home/user/go/pkg/mod/google.golang.org/grpc@v1.38.0/server.go:934 +0x98
created by google.golang.org/grpc.(*Server).serveStreams.func1
    /home/user/go/pkg/mod/google.golang.org/grpc@v1.38.0/server.go:932 +0x28a

2022-04-22 11:59:16.234 [CRT] RPCS: Stack trace: goroutine 430 [running]:
runtime/debug.Stack()
    /usr/lib/go-1.18/src/runtime/debug/stack.go:24 +0x65
github.com/kaspanet/kaspad/util/panics.HandlePanic(0xc01a4e7ec0?, {0xc024b4ed50, 0x2c}, {0xc015090000, 0x943, 0x1000})
    /home/user/Coding/kaspad/kaspad/util/panics/panics.go:22 +0xf4
panic({0xb07f00, 0xc0187b1030})
    /usr/lib/go-1.18/src/runtime/panic.go:838 +0x207
github.com/kaspanet/kaspad/app/rpc.(*Manager).handleError(0xc01a5b4600?, {0xd20f60?, 0xc0187b1030}, 0xc0220460f0)
    /home/user/Coding/kaspad/kaspad/app/rpc/rpc.go:104 +0x109
github.com/kaspanet/kaspad/app/rpc.(*Manager).routerInitializer.func1()
    /home/user/Coding/kaspad/kaspad/app/rpc/rpc.go:69 +0x9a
github.com/kaspanet/kaspad/util/panics.handleSpawnedFunction(0xc0001c9800, {0xc015090000, 0x943, 0x1000}, {0xc2abd3?, 0xc02fac0c00?}, 0xc0220461e0)
    /home/user/Coding/kaspad/kaspad/util/panics/panics.go:83 +0x26a
github.com/kaspanet/kaspad/util/panics.GoroutineWrapperFunc.func1.1()
    /home/user/Coding/kaspad/kaspad/util/panics/panics.go:32 +0x35
created by github.com/kaspanet/kaspad/util/panics.GoroutineWrapperFunc.func1
    /home/user/Coding/kaspad/kaspad/util/panics/panics.go:31 +0xf8
cdnsteve commented 2 years ago

Confirmed here too, was able to reproduce this on Win 10:

c:\Mining\kaspad-v0.12-mainnet>kaspactl.exe GetBalanceByAddress
error parsing command: command 'GetBalanceByAddress' expects 1 parameters but got 0

c:\Mining\kaspad-v0.12-mainnet>kaspactl.exe GetBalanceByAddress 1
error posting the request to the RPC server: error receiving the response from the RPC server: rpc error: code = Unavailable desc = error reading from server: read tcp [::1]:58605->[::1]:16110: wsarecv: An existing connection was forcibly closed by the remote host.

Note the above seems to check for input in the first try but doesn't validate format in the second attempt.

This immediately crashed the kaspad service running.

Error output in that terminal:

2022-04-25 23:24:05.899 [INF] TXMP: RPC Incoming connection from [::1]:58605
2022-04-25 23:24:05.906 [CRT] RPCS: Exiting: Fatal error in goroutine `routerInitializer-handleIncomingMessages 1128`: Couldn't decode address '1': decoded address is of unknown format: invalid bech32 string length 1
2022-04-25 23:24:05.906 [CRT] RPCS: Goroutine stack trace: goroutine 3794 [running]:
runtime/debug.Stack()
        C:/hostedtoolcache/windows/go/1.18.0/x64/src/runtime/debug/stack.go:24 +0x65
github.com/kaspanet/kaspad/util/panics.GoroutineWrapperFunc.func1({0x17e02b0, 0x28}, 0xc06a7fac00)
        D:/a/kaspad/kaspad/util/panics/panics.go:30 +0x35
github.com/kaspanet/kaspad/app/rpc.(*Manager).routerInitializer(0xc01c44a368, 0xc06a7faae0, 0xc06a7faba0)
        D:/a/kaspad/kaspad/app/rpc/rpc.go:65 +0x222
github.com/kaspanet/kaspad/infrastructure/network/netadapter.newNetConnection({0x18e5640?, 0xc05632e700}, 0xc01c425f30, {0x17c8dee?, 0x60?})
        D:/a/kaspad/kaspad/infrastructure/network/netadapter/netconnection.go:41 +0xf2
github.com/kaspanet/kaspad/infrastructure/network/netadapter.(*NetAdapter).onRPCConnectedHandler(0x68?, {0x18e5640?, 0xc05632e700?})
        D:/a/kaspad/kaspad/infrastructure/network/netadapter/netadapter.go:151 +0x3c
github.com/kaspanet/kaspad/infrastructure/network/netadapter/server/grpcserver.(*gRPCServer).handleInboundConnection(0xc00077adc0, {0x18e09f8, 0xc06a7faab0}, {0x1e8445fc108?, 0xc0661704f0})
        D:/a/kaspad/kaspad/infrastructure/network/netadapter/server/grpcserver/grpc_server.go:113 +0x18b
github.com/kaspanet/kaspad/infrastructure/network/netadapter/server/grpcserver.(*rpcServer).MessageStream(0xc00077adc0, {0x18e5748, 0xc0661704f0})
        D:/a/kaspad/kaspad/infrastructure/network/netadapter/server/grpcserver/rpcserver.go:31 +0xbe
github.com/kaspanet/kaspad/infrastructure/network/netadapter/server/grpcserver/protowire._RPC_MessageStream_Handler({0x1766360?, 0xc00077adc0}, {0x18e4228?, 0xc0107883c0})
        D:/a/kaspad/kaspad/infrastructure/network/netadapter/server/grpcserver/protowire/messages_grpc.pb.go:214 +0x9f
google.golang.org/grpc.(*Server).processStreamingRPC(0xc000155c00, {0x18e5a68, 0xc07d839c80}, 0xc02c2fe480, 0xc00077fb00, 0x1613480, 0x0)
        C:/Users/runneradmin/go/pkg/mod/google.golang.org/grpc@v1.38.0/server.go:1533 +0xd06
google.golang.org/grpc.(*Server).handleStream(0xc000155c00, {0x18e5a68, 0xc07d839c80}, 0xc02c2fe480, 0x0)
        C:/Users/runneradmin/go/pkg/mod/google.golang.org/grpc@v1.38.0/server.go:1613 +0x9d6
google.golang.org/grpc.(*Server).serveStreams.func1.2()
        C:/Users/runneradmin/go/pkg/mod/google.golang.org/grpc@v1.38.0/server.go:934 +0x98
created by google.golang.org/grpc.(*Server).serveStreams.func1
        C:/Users/runneradmin/go/pkg/mod/google.golang.org/grpc@v1.38.0/server.go:932 +0x28a

2022-04-25 23:24:05.906 [CRT] RPCS: Stack trace: goroutine 3795 [running]:
runtime/debug.Stack()
        C:/hostedtoolcache/windows/go/1.18.0/x64/src/runtime/debug/stack.go:24 +0x65
github.com/kaspanet/kaspad/util/panics.HandlePanic(0xc01c42a9c0?, {0xc04a0ebf50, 0x2d}, {0xc039be4000, 0x948, 0x1000})
        D:/a/kaspad/kaspad/util/panics/panics.go:22 +0xf4
panic({0x16bcdc0, 0xc04b508180})
        C:/hostedtoolcache/windows/go/1.18.0/x64/src/runtime/panic.go:838 +0x207
github.com/kaspanet/kaspad/app/rpc.(*Manager).handleError(0xc01c44a368?, {0x18d8ba0?, 0xc04b508180}, 0xc06a7faba0)
        D:/a/kaspad/kaspad/app/rpc/rpc.go:104 +0x109
github.com/kaspanet/kaspad/app/rpc.(*Manager).routerInitializer.func1()
        D:/a/kaspad/kaspad/app/rpc/rpc.go:69 +0x9a
github.com/kaspanet/kaspad/util/panics.handleSpawnedFunction(0xc00015dda0, {0xc039be4000, 0x948, 0x1000}, {0x17e02b0?, 0xc02b2ad2a0?}, 0xc06a7fac00)
        D:/a/kaspad/kaspad/util/panics/panics.go:83 +0x26a
github.com/kaspanet/kaspad/util/panics.GoroutineWrapperFunc.func1.1()
        D:/a/kaspad/kaspad/util/panics/panics.go:32 +0x35
created by github.com/kaspanet/kaspad/util/panics.GoroutineWrapperFunc.func1
        D:/a/kaspad/kaspad/util/panics/panics.go:31 +0xf8
cdnsteve commented 2 years ago

This also makes me think about limiting specific commands/whitelist ones if a Pool is running the service or someone is exposing their node externally as a future feature.

D-Stacks commented 2 years ago

I think the easiest and fastest way to test for these types of bugs is probably to ensure that all of the rpc handling functions do not return an error.

cdnsteve commented 2 years ago

It looks like this has been resolved

c:\Mining\kaspad-v0.12.1-mainnet>kaspactl.exe GetBalanceByAddress
error parsing command: command 'GetBalanceByAddress' expects 1 parameters but got 0

c:\Mining\kaspad-v0.12.1-mainnet>kaspactl.exe GetBalanceByAddress 1
{
    "getUtxosByAddressesResponse":  {
        "entries":  [],
        "error":  {
            "message":  "Couldn't decode address '1': decoded address is of unknown format: invalid bech32 string length 1"
        }
    }
}

c:\Mining\kaspad-v0.12.1-mainnet>kaspactl.exe GetBalanceByAddress 1--40425-0542
{
    "getUtxosByAddressesResponse":  {
        "entries":  [],
        "error":  {
            "message":  "Couldn't decode address '1--40425-0542': decoded address is of unknown format: invalid index of ':'"
        }
    }
}
D-Stacks commented 2 years ago

It looks like this has been resolved

c:\Mining\kaspad-v0.12.1-mainnet>kaspactl.exe GetBalanceByAddress
error parsing command: command 'GetBalanceByAddress' expects 1 parameters but got 0

c:\Mining\kaspad-v0.12.1-mainnet>kaspactl.exe GetBalanceByAddress 1
{
    "getUtxosByAddressesResponse":  {
        "entries":  [],
        "error":  {
            "message":  "Couldn't decode address '1': decoded address is of unknown format: invalid bech32 string length 1"
        }
    }
}

c:\Mining\kaspad-v0.12.1-mainnet>kaspactl.exe GetBalanceByAddress 1--40425-0542
{
    "getUtxosByAddressesResponse":  {
        "entries":  [],
        "error":  {
            "message":  "Couldn't decode address '1--40425-0542': decoded address is of unknown format: invalid index of ':'"
        }
    }
}

Yes, will close.