Make sure all ports are 16 bit numbers #8394
Conversation
src/test/netbase_tests.cpp
Outdated
There was a problem hiding this comment.
This needs a larger type because it can be -1, which is not equal to 65535.
There was a problem hiding this comment.
True! Changed to use 0 instead.
|
This probably overlaps a lot with @theuni's net code overhaul. |
|
Addressed test issue and changed |
|
yes, this conflicts somewhat, mostly in net/netbase. I can deal with that though. More concerning though is the lack over overflow handling on input in a few places. For example: what will "-rpcport=65537" do? I see a few of those, though admittedly some of them likely already exist. Also looks like a few are fixed, though :) @UdjinM6 Would you mind first ensuring that all input ports are sanitized (including those coming over the net) before doing the replacement? |
|
Thanks for the review @theuni ! Meanwhile:
I'm not sure which parts of code you were referencing by
Could you provide some guidance pls? |
|
Needs rebase after #8128 |
|
Rebased and (re)added fixes to new code and to code which was moved in #8128. |
src/bitcoin-cli.cpp
Outdated
| std::string host = GetArg("-rpcconnect", DEFAULT_RPCCONNECT); | ||
| int port = GetArg("-rpcport", BaseParams().RPCPort()); | ||
|
|
||
| int portTmp = GetArg("-rpcport", BaseParams().RPCPort()); |
There was a problem hiding this comment.
Nit: GetArg returns int64_t. I think the goal of the pull was not to cast without check?
|
To be honest, I'm not entirely convinced that merging this is worth the risk (smaller integers are easier to overflow, for example). |
- use int64_t to get result from GetArg - use int64_t for IsPortValid() - remove manual numbers conversion to uint16_t
|
@laanwj Agree that there are a lot of changes, but I think in the long run we should make sure that arguments are parsed correctly and sanitized. I think this pull improves parsing of port numbers passed in by users. |
|
Improving parsing is a good thing, if we can factor that out I'd have no problem with it. |
|
Closing this for now. |
Ports should always be 16 bit numbers so use proper type (
uint16_t) and format (%u).Also changing
unsigned shorttouint16_tfor such cases for consistency.Reference: https://tools.ietf.org/html/rfc793#section-3.1