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.

Sign up for GitHub

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

Jump to bottom

init: torcontrol argument should be validated #23589

Open
laanwj opened this issue Nov 25, 2021 · 9 comments
Open

init: torcontrol argument should be validated #23589

laanwj opened this issue Nov 25, 2021 · 9 comments
Labels
good first issue P2P

Comments

@laanwj
Copy link
Member

laanwj commented Nov 25, 2021

I accidentally -torcontrol=1 today (instead of -listenonion=1) and was confused that it was accepted, as the argument needs to be host:port.

Expected outcome would be an error message and exiting.

Useful skills:

  • C++
  • Understanding of bitcoin core's initialization sequence

Want to work on this issue?

For guidance on contributing, please read CONTRIBUTING.md before opening your pull request.
@shaavan
Copy link
Contributor

shaavan commented Nov 25, 2021

Let me try tackling this!

@shaavan
Copy link
Contributor

shaavan commented Nov 25, 2021

Ok, so I was able to add a function successfully that can check if the given string is a valid <host>:<port> pair or not. The issue is that I cannot raise an error when the argument is wrongly passed when initializing bitcoind.

In fact, bitcoind is not raising an error for any invalid argument passed at initialization.

Screenshot from 2021-11-25 18-43-22

Expected outcome would be an error message and exiting.

Just a confirmation. When the ./src/bitcoind command is initially run with invalid parameters, do we expect an error message and exit? Or is it expected to occur later in the runtime of bitcoind?

@amovfx
Copy link

amovfx commented Nov 25, 2021

Is the Args Manager flags supposed to help with this?

@ityodadev
Copy link

common sense would suggest there should be a message and some exit status, other than 0

@shaavan
Copy link
Contributor

shaavan commented Nov 26, 2021

A gentle ping, @laanwj. Would you please take a look at this comment?

@maflcko
Copy link
Member

maflcko commented Nov 26, 2021

git grep InitError might help you.

@shaavan shaavan mentioned this issue Nov 26, 2021
Closed
@bitcoin bitcoin deleted a comment from Wonders11 Nov 27, 2021
@bitcoin bitcoin deleted a comment from ComplaintID Nov 28, 2021
@uri44derr13 uri44derr13 mentioned this issue Dec 19, 2021
Closed
@Abbasshafiei Abbasshafiei mentioned this issue Dec 30, 2021
Closed
@bitcoin bitcoin deleted a comment Jan 5, 2022
@dakshp07
Copy link

dakshp07 commented Mar 4, 2022 •
edited

hey folks, I'm very new to the codebase and wanted to try my hands around this issue. Also, I saw a PR that is targeting this issue so what's the update on that, is the issue still open?

@bitcoin bitcoin deleted a comment from TeresaHodges764 Mar 10, 2022
amadeuszpawlik added a commit to amadeuszpawlik/bitcoin that referenced this issue May 14, 2022
@amadeuszpawlik
Ensure valid socket address in -torcontrol
b8cb03f 
- Adds `util/network` with a helper function
`hasValidHostPort(socketAddr)` for easy checking.
The only checks for `hostname` are: not empty, and no spaces.
- Returns error at init if `-torcontrol` is not a valid host:port combo.
- `StartTorControl` returns a `bool`, and has a return parameter `error`
where a suitable message is set at failure.

Fixes bitcoin#23589
amadeuszpawlik added a commit to amadeuszpawlik/bitcoin that referenced this issue May 14, 2022
@amadeuszpawlik
Ensure valid socket address in -torcontrol
79befd6 
- Adds `util/network` with a helper function
`hasValidHostPort(socketAddr)` for easy checking.
The only checks for `hostname` are: not empty, and no spaces.
- Returns error at init if `-torcontrol` is not a valid host:port combo.
- `StartTorControl` returns a `bool`, and has a return parameter `error`
where a suitable message is set at failure.

Fixes bitcoin#23589
amadeuszpawlik added a commit to amadeuszpawlik/bitcoin that referenced this issue May 14, 2022
@amadeuszpawlik
Ensure valid socket address in -torcontrol
69d4d76 
- Returns error at init if `-torcontrol` is not a valid host:port combo.
- `StartTorControl` returns a `bool`, and has a return parameter `error`
where a suitable message is set at failure.

Fixes bitcoin#23589
@amadeuszpawlik amadeuszpawlik mentioned this issue May 14, 2022
Closed
amadeuszpawlik added a commit to amadeuszpawlik/bitcoin that referenced this issue May 26, 2022
@amadeuszpawlik
Ensure valid socket address in -torcontrol
f48c207 
- Returns error at init if `-torcontrol` is not a valid host:port combo.
- `StartTorControl` returns a `bool`, and has a return parameter `error`
where a suitable message is set at failure.

Fixes bitcoin#23589
@bitcoin bitcoin deleted a comment from yndue736 May 31, 2022
amadeuszpawlik added a commit to amadeuszpawlik/bitcoin that referenced this issue Oct 13, 2022
@amadeuszpawlik
Ensure valid socket address in -torcontrol
8695e9a 
- Returns error at init if `-torcontrol` is not a valid host:port combo.
- `StartTorControl` returns a `bool`, and has a return parameter `error`
where a suitable message is set at failure.

Fixes bitcoin#23589
amadeuszpawlik added a commit to amadeuszpawlik/bitcoin that referenced this issue Oct 13, 2022
@amadeuszpawlik
Ensure valid network address in -torcontrol
e0a81da 
- Returns error at init if `-torcontrol` is not a valid host:port combo.
- `StartTorControl` returns a `bool`, and has a return parameter `error`
where a suitable message is set at failure.

Fixes bitcoin#23589
amadeuszpawlik added a commit to amadeuszpawlik/bitcoin that referenced this issue Oct 13, 2022
@amadeuszpawlik
Ensure valid network address in -torcontrol
7ac9da5 
- Returns error at init if `-torcontrol` is not a valid host:port combo.
- `StartTorControl` returns a `bool`, and has a return parameter `error`
where a suitable message is set at failure.

Fixes bitcoin#23589
amadeuszpawlik added a commit to amadeuszpawlik/bitcoin that referenced this issue Oct 16, 2022
@amadeuszpawlik
Ensure valid network address in -torcontrol
5be4336 
- Returns error at init if `-torcontrol` is not a valid host:port combo.
- `StartTorControl` returns a `bool`, and has a return parameter `error`
where a suitable message is set at failure.

Fixes bitcoin#23589
vstoyanov pushed a commit to vstoyanov/bitcoin that referenced this issue Apr 11, 2023
@amadeuszpawlik
Ensure valid network address in -torcontrol
88168de 
- Returns error at init if `-torcontrol` is not a valid host:port combo.
- `StartTorControl` returns a `bool`, and has a return parameter `error`
where a suitable message is set at failure.

Fixes bitcoin#23589
vstoyanov pushed a commit to vstoyanov/bitcoin that referenced this issue Apr 11, 2023
@amadeuszpawlik
Ensure valid network address in -torcontrol
6fb9b72 
- Returns error at init if `-torcontrol` is not a valid host:port combo.
- `StartTorControl` returns a `bool`, and has a return parameter `error`
where a suitable message is set at failure.

Fixes bitcoin#23589
@kevkevinpal
Copy link
Contributor

I see it not throwing the error because it is returning true here if it has no :

bitcoin/src/util/strencodings.cpp

Line 117 in 61d59fe

valid = true;

I can open a PR to change this to false but it would affect these files that use SplitHostPort in ./bitcoin/src

./src/test/fuzz/string.cpp:87
./src/test/netbase_tests.cpp:89
./src/util/strencodings.cpp:102
./src/util/strencodings.h:106
./src/bitcoin-cli.cpp:746
./src/net.cpp:547
./src/netbase.cpp:189
./src/init.cpp:1265
./src/init.cpp:1279

We could also do a check just for -torcontrol to check if the : exists and if it doesn't then throw an error
@laanwj @MarcoFalke what do you think?

@kevkevinpal kevkevinpal mentioned this issue Jun 30, 2023
Closed
@kevkevinpal
Copy link
Contributor

does this PR close this issue?
#28101

bufo24 pushed a commit to bufo24/bitcoin that referenced this issue Oct 27, 2023
@amadeuszpawlik @bufo24
Ensure valid network address in -torcontrol
fd09b3e 
- Returns error at init if `-torcontrol` is not a valid host:port combo.
- `StartTorControl` returns a `bool`, and has a return parameter `error`
where a suitable message is set at failure.

Fixes bitcoin#23589
bufo24 pushed a commit to bufo24/bitcoin that referenced this issue Oct 27, 2023
@amadeuszpawlik @bufo24
Ensure valid network address in -torcontrol
2d15e76 
- Returns error at init if `-torcontrol` is not a valid host:port combo.
- `StartTorControl` returns a `bool`, and has a return parameter `error`
where a suitable message is set at failure.

Fixes bitcoin#23589
@bufo24 bufo24 mentioned this issue Nov 2, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue P2P
Projects
None yet
Milestone
No milestone
8 participants
@laanwj @maflcko @kevkevinpal @amovfx @dakshp07 @ityodadev @shaavan and others