net: Don't allow resolving of std::string with embedded NUL characters. Add tests. #17754

practicalswift · 2019-12-16T13:41:59Z

Don't allow resolving of std::string:s with embedded NUL characters.

Avoid using C-style NUL-terminated strings as arguments in the netbase interface

Add tests.

The only place in where C-style NUL-terminated strings are actually needed is here:

+    if (!ValidAsCString(name)) {
+        return false;
+    }
...
-    int nErr = getaddrinfo(pszName, nullptr, &aiHint, &aiRes);
+    int nErr = getaddrinfo(name.c_str(), nullptr, &aiHint, &aiRes);
     if (nErr)
         return false;

Interface changes:

-bool LookupHost(const char *pszName, std::vector<CNetAddr>& vIP, unsigned int nMaxSolutions, bool fAllowLookup);
+bool LookupHost(const std::string& name, std::vector<CNetAddr>& vIP, unsigned int nMaxSolutions, bool fAllowLookup);

-bool LookupHost(const char *pszName, CNetAddr& addr, bool fAllowLookup);
+bool LookupHost(const std::string& name, CNetAddr& addr, bool fAllowLookup);

-bool Lookup(const char *pszName, CService& addr, int portDefault, bool fAllowLookup);
+bool Lookup(const std::string& name, CService& addr, int portDefault, bool fAllowLookup);

-bool Lookup(const char *pszName, std::vector<CService>& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions);
+bool Lookup(const std::string& name, std::vector<CService>& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions);

-bool LookupSubNet(const char *pszName, CSubNet& subnet);
+bool LookupSubNet(const std::string& strSubnet, CSubNet& subnet);

-CService LookupNumeric(const char *pszName, int portDefault = 0);
+CService LookupNumeric(const std::string& name, int portDefault = 0);

-bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int port, const SOCKET& hSocketRet, int nTimeout, bool *outProxyConnectionFailed);
+bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int port, const SOCKET& hSocketRet, int nTimeout, bool& outProxyConnectionFailed);

It should be noted that the ConnectThroughProxy change (from bool *outProxyConnectionFailed to bool& outProxyConnectionFailed) has nothing to do with NUL handling but I thought it was worth doing when touching this file :)

DrahtBot · 2019-12-16T15:31:20Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#17602 (net: Limit # of IPs learned from a DNS seed by family by dongcarl)
#17356 (RPC: Internal named params by luke-jr)
#17160 (refactor: net: subnet lookup: use single-result LookupHost() by theStack)
#16548 (Make the global flag fDiscover an instance variable of CConnman by mmachicao)
#16224 (gui: Bilingual GUI error messages by hebasto)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

elichai · 2019-12-16T17:14:35Z

Concept ACK. It's best to not use C style Strings whenever it's possible, and if and when it's required we should add sanity checks.

instagibbs · 2019-12-16T19:03:11Z

utACK a32d3cc

I always learn something new about implicit conversion in C++

laanwj · 2019-12-17T08:50:55Z

Concept ACK
Also gets rid of a lot of the remaining (unnecessary) c_str().
This is something I've been wanting to do for a long time but never dared.

promag

Code review ACK a32d3cc.

src/test/netbase_tests.cpp

fanquake · 2019-12-29T16:22:01Z

cc @EthanHeilman

promag

ACK 10ef76f.

src/test/netbase_tests.cpp

EthanHeilman

ACK clear code base improvement

…etbase interface

…meters with embedded NUL characters

practicalswift · 2020-01-08T12:37:00Z

Rebased to get rid of non-related Travis macOS failure!

@promag @EthanHeilman Please re-review :)

src/net.cpp

EthanHeilman · 2020-01-08T20:01:25Z

ACK 7a046cd
I did not run any tests. I read through the code carefully, looking up behavior I didn't understand. I don't see any reasons not to merge.

laanwj · 2020-01-22T19:20:35Z

ACK 7a046cd

It should be noted that the ConnectThroughProxy change (from bool *outProxyConnectionFailed to bool& outProxyConnectionFailed) has nothing to do with NUL handling but I thought it was worth doing when touching this file :)

FWIW I think such unrelated changes should generally be avoided (especially as part of the same commit). But I don't mind it in this case, personally, and this has had enough review.

… NUL characters. Add tests. 7a046cd tests: Avoid using C-style NUL-terminated strings as arguments (practicalswift) fefb916 tests: Add tests to make sure lookup methods fail on std::string parameters with embedded NUL characters (practicalswift) 9574de8 net: Avoid using C-style NUL-terminated strings as arguments in the netbase interface (practicalswift) Pull request description: Don't allow resolving of `std::string`:s with embedded `NUL` characters. Avoid using C-style `NUL`-terminated strings as arguments in the `netbase` interface Add tests. The only place in where C-style `NUL`-terminated strings are actually needed is here: ```diff + if (!ValidAsCString(name)) { + return false; + } ... - int nErr = getaddrinfo(pszName, nullptr, &aiHint, &aiRes); + int nErr = getaddrinfo(name.c_str(), nullptr, &aiHint, &aiRes); if (nErr) return false; ``` Interface changes: ```diff -bool LookupHost(const char *pszName, std::vector<CNetAddr>& vIP, unsigned int nMaxSolutions, bool fAllowLookup); +bool LookupHost(const std::string& name, std::vector<CNetAddr>& vIP, unsigned int nMaxSolutions, bool fAllowLookup); -bool LookupHost(const char *pszName, CNetAddr& addr, bool fAllowLookup); +bool LookupHost(const std::string& name, CNetAddr& addr, bool fAllowLookup); -bool Lookup(const char *pszName, CService& addr, int portDefault, bool fAllowLookup); +bool Lookup(const std::string& name, CService& addr, int portDefault, bool fAllowLookup); -bool Lookup(const char *pszName, std::vector<CService>& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions); +bool Lookup(const std::string& name, std::vector<CService>& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions); -bool LookupSubNet(const char *pszName, CSubNet& subnet); +bool LookupSubNet(const std::string& strSubnet, CSubNet& subnet); -CService LookupNumeric(const char *pszName, int portDefault = 0); +CService LookupNumeric(const std::string& name, int portDefault = 0); -bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int port, const SOCKET& hSocketRet, int nTimeout, bool *outProxyConnectionFailed); +bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int port, const SOCKET& hSocketRet, int nTimeout, bool& outProxyConnectionFailed); ``` It should be noted that the `ConnectThroughProxy` change (from `bool *outProxyConnectionFailed` to `bool& outProxyConnectionFailed`) has nothing to do with `NUL` handling but I thought it was worth doing when touching this file :) ACKs for top commit: EthanHeilman: ACK 7a046cd laanwj: ACK 7a046cd Tree-SHA512: 66556e290db996917b54091acd591df221f72230f6b9f6b167b9195ee870ebef6e26f4cda2f6f54d00e1c362e1743bf56785d0de7cae854e6bf7d26f6caccaba

…mbedded NUL characters. Add tests. 7a046cd tests: Avoid using C-style NUL-terminated strings as arguments (practicalswift) fefb916 tests: Add tests to make sure lookup methods fail on std::string parameters with embedded NUL characters (practicalswift) 9574de8 net: Avoid using C-style NUL-terminated strings as arguments in the netbase interface (practicalswift) Pull request description: Don't allow resolving of `std::string`:s with embedded `NUL` characters. Avoid using C-style `NUL`-terminated strings as arguments in the `netbase` interface Add tests. The only place in where C-style `NUL`-terminated strings are actually needed is here: ```diff + if (!ValidAsCString(name)) { + return false; + } ... - int nErr = getaddrinfo(pszName, nullptr, &aiHint, &aiRes); + int nErr = getaddrinfo(name.c_str(), nullptr, &aiHint, &aiRes); if (nErr) return false; ``` Interface changes: ```diff -bool LookupHost(const char *pszName, std::vector<CNetAddr>& vIP, unsigned int nMaxSolutions, bool fAllowLookup); +bool LookupHost(const std::string& name, std::vector<CNetAddr>& vIP, unsigned int nMaxSolutions, bool fAllowLookup); -bool LookupHost(const char *pszName, CNetAddr& addr, bool fAllowLookup); +bool LookupHost(const std::string& name, CNetAddr& addr, bool fAllowLookup); -bool Lookup(const char *pszName, CService& addr, int portDefault, bool fAllowLookup); +bool Lookup(const std::string& name, CService& addr, int portDefault, bool fAllowLookup); -bool Lookup(const char *pszName, std::vector<CService>& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions); +bool Lookup(const std::string& name, std::vector<CService>& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions); -bool LookupSubNet(const char *pszName, CSubNet& subnet); +bool LookupSubNet(const std::string& strSubnet, CSubNet& subnet); -CService LookupNumeric(const char *pszName, int portDefault = 0); +CService LookupNumeric(const std::string& name, int portDefault = 0); -bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int port, const SOCKET& hSocketRet, int nTimeout, bool *outProxyConnectionFailed); +bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int port, const SOCKET& hSocketRet, int nTimeout, bool& outProxyConnectionFailed); ``` It should be noted that the `ConnectThroughProxy` change (from `bool *outProxyConnectionFailed` to `bool& outProxyConnectionFailed`) has nothing to do with `NUL` handling but I thought it was worth doing when touching this file :) ACKs for top commit: EthanHeilman: ACK 7a046cd laanwj: ACK 7a046cd Tree-SHA512: 66556e290db996917b54091acd591df221f72230f6b9f6b167b9195ee870ebef6e26f4cda2f6f54d00e1c362e1743bf56785d0de7cae854e6bf7d26f6caccaba

Summary: > Don't allow resolving of std::string:s with embedded NUL characters. > Avoid using C-style NUL-terminated strings as arguments in the netbase interface > Add tests. This is a backport of [[bitcoin/bitcoin#17754 | PR17754]] [1/3] bitcoin/bitcoin@9574de8 Test Plan: `ninja && ninja check` Reviewers: #bitcoin_abc, majcosta Reviewed By: #bitcoin_abc, majcosta Differential Revision: https://reviews.bitcoinabc.org/D8684

…racters Summary: > Add tests to make sure lookup methods fail on std::string parameters with embedded NUL characters This is a backport of Core [[bitcoin/bitcoin#17754 | PR17754]] [2/3] bitcoin/bitcoin@fefb916 Depends on D8684 Test Plan: `ninja && ninja check` Reviewers: #bitcoin_abc, Fabien, majcosta Reviewed By: #bitcoin_abc, Fabien, majcosta Differential Revision: https://reviews.bitcoinabc.org/D8685

Summary: This is a backport of Core [[bitcoin/bitcoin#17754 | PR17754]] [3/3] bitcoin/bitcoin@7a046cd Depends on D8685 Test Plan: `ninja && ninja check` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D8686

ecde04a [Consensus] Bump Active Protocol version to 70923 for v5.3 (random-zebra) b63e4f5 Consensus: Add v5.3 enforcement height for testnet. (furszy) f44be94 Only relay IPv4, IPv6, Tor addresses (Pieter Wuille) 015298c fix: tor: Call event_base_loopbreak from the event's callback (furszy) 34ff7a8 Consensus: Add mnb ADDRv2 guard. (furszy) b4515dc GUI: Present v3 onion addresses properly in MNs list. (furszy) 337d43d tests: don't export in6addr_loopback (Vasil Dimov) 2cde8e0 GUI: Do not show the tor v3 onion address in the topbar. (furszy) 0b5f406 Doc: update tor.md with latest upstream information. (furszy) 89df7f2 addrman: ensure old versions don't parse peers.dat (Vasil Dimov) bb90c5c test: add getnetworkinfo network name regression tests (Jon Atack) d8e01b5 rpc: update GetNetworksInfo() to not return unsupported networks (Jon Atack) 57fc7b0 net: update GetNetworkName() with all enum Network cases (Jon Atack) 647d60b tests: Modify rpc_bind to conform to bitcoin#14532 behaviour. (Carl Dong) d4d6729 Allow running rpc_bind.py --nonloopback test without IPv6 (Kristaps Kaupe) 4a034d8 test: Add rpc_bind test to default-run tests (Wladimir J. van der Laan) 61a08af [tests] bind functional test nodes to 127.0.0.1 Prevents OSX firewall (Sjors Provoost) 6a4f1e0 test: Add basic addr relay test (furszy) 78aa61c net: Make addr relay mockable (furszy) ba954ca Send and require SENDADDRV2 before VERACK (Pieter Wuille) 61c2ed4 Bump net protocol version + don't send 'sendaddrv2' to pre-70923 software (furszy) ccd508a tor: make a TORv3 hidden service instead of TORv2 (Vasil Dimov) 6da9a14 net: advertise support for ADDRv2 via new message (furszy) e58d5d0 Migrate to test_large_inv() to Misbehaving logging. (furszy) d496b64 [QA] fix mininode CAddress ser/deser (Jonas Schnelli) cec9567 net: CAddress & CAddrMan: (un)serialize as ADDRv2 Change the serialization of `CAddrMan` to serialize its addresses in ADDRv2/BIP155 format by default. Introduce a new `CAddrMan` format version (3). (furszy) b8c1dda streams update: get rid of nType and nVersion. (furszy) 3eaa273 Support bypassing range check in ReadCompactSize (Pieter Wuille) a237ba4 net: recognize TORv3/I2P/CJDNS networks (Vasil Dimov) 8e50853 util: make EncodeBase32 consume Spans (Sebastian Falbesoner) 1f67e30 net: CNetAddr: add support to (un)serialize as ADDRv2 (Vasil Dimov) 2455420 test: move HasReason so it can be reused (furszy) d41adb4 util: move HasPrefix() so it can be reused (Vasil Dimov) f6f86af Unroll Keccak-f implementation (Pieter Wuille) 45222e6 Implement keccak-f[1600] and SHA3-256 (Pieter Wuille) 08ad06d net: change CNetAddr::ip to have flexible size (furszy) 3337219 net: improve encapsulation of CNetAddr. (furszy) 910d5c4 test: Do not instantiate CAddrDB for static call (Hennadii Stepanov) 6b607ef Drop IsLimited in favor of IsReachable (Ben Woosley) a40711b IsReachable is the inverse of IsLimited (DRY). Includes unit tests (marcaiaf) 8839828 net: don't accept non-left-contiguous netmasks (Vasil Dimov) 5d7f864 rpcbind: Warn about exposing RPC to untrusted networks (Luke Dashjr) 2a6abd8 CNetAddr: Add IsBindAny method to check for INADDR_ANY (Luke Dashjr) 4fdfa45 net: Always default rpcbind to localhost, never "all interfaces" (Luke Dashjr) 31064a8 net: Minor accumulated cleanups (furszy) 9f9c871 tests: Avoid using C-style NUL-terminated strings as arguments (practicalswift) f6c52a3 tests: Add tests to make sure lookup methods fail on std::string parameters with embedded NUL characters (practicalswift) a751b9b net: Avoid using C-style NUL-terminated strings as arguments in the netbase interface (furszy) f30869d test: add IsRFC2544 tests (Mark Tyneway) ed5abe1 Net: Proper CService deserialization + GetIn6Addr return false if addr isn't an IPv6 addr (furszy) 86d73fb net: save the network type explicitly in CNetAddr (Vasil Dimov) ad57dfc net: document `enum Network` (Vasil Dimov) cb160de netaddress: Update CNetAddr for ORCHIDv2 (Carl Dong) c3c04e4 net: Better misbehaving logging (furszy) 3660487 net: Use C++11 member initialization in protocol (Marco) 082baa3 refactor: Drop unused CBufferedFile::Seek() (Hennadii Stepanov) e2d776a util: CBufferedFile fixes (Larry Ruane) 6921f42 streams: backport OverrideStream class (furszy) Pull request description: Conjunction of a large number of back ports, updates and refactorings that made with the final goal of implementing v3 Onion addresses support (BIP155 https://github.com/bitcoin/bips/blob/master/bip-0155.mediawiki) before the tor v2 addresses EOL, scheduled, by the Tor project, for (1) July 15th: v2 addr support removal from the code base, and (2) October 15th: v2 addr network disable, where **every peer in our network running under Tor will loose the connection and drop the network**. As BIP155 describes, this is introducing a new P2P message to gossip longer node addresses over the P2P network. This is required to support new-generation Onion addresses, I2P, and potentially other networks that have longer endpoint addresses than fit in the 128 bits of the current addr message. In order to achieve the end goal, had to: 1. Create Span class and push it up to latest Bitcoin implementation. 2. Update the whole serialization framework and every object using it up to latest Bitcoin implementation (3-4 years ahead of what we currently are in master). 3. Update the address manager implementing ASN-based bucketing of the network nodes. 4. Update and refactor the netAddress and address manager tests to latest Bitcoin implementation (4 years ahead of what we currently are in master). 5. Several util string, vector, encodings, parsing, hashing backports and more.. Important note: This PR it is not meant to be merged as a standalone PR, will decouple smaller ones moving on. Adding on each sub-PR its own description isolated from this big monster. Second note: This is still a **work-in-progress**, not ready for testing yet. I'm probably missing to mention few PRs that have already adapted to our sources. Just making it public so can decouple the changes, we can start merging them and i can continue working a bit more confortable (rebase a +170 commits separate branch is not fun..). ### List of back ported and adapted PRs: Span and Serialization: ---------------- * bitcoin#12886. * bitcoin#12916. * bitcoin#13558. * bitcoin#13697. (Only Span's commit 29943a9) * bitcoin#17850. * bitcoin#17896. * bitcoin#12752. * bitcoin#16577. * bitcoin#16670. (without faebf62) * bitcoin#17957. * bitcoin#18021. * bitcoin#18087. * bitcoin#18112 (only from 353f376 that we don't support). * bitcoin#18167. * bitcoin#18317. * bitcoin#18591 (only Span's commit 0fbde48) * bitcoin#18468. * bitcoin#19020. * bitcoin#19032. * bitcoin#19367. * bitcoin#19387. Net, NetAddress and AddrMan: ---------------- * bitcoin#7932. * bitcoin#10756. * bitcoin#10765. * bitcoin#12218. * bitcoin#12855. * bitcoin#13532. * bitcoin#13575. * bitcoin#13815. * bitcoin#14532. * bitcoin#15051. * bitcoin#15138. * bitcoin#15689. * bitcoin#16702. * bitcoin#17243. * bitcoin#17345. * bitcoin#17754. * bitcoin#17758. * bitcoin#17812. * bitcoin#18023. * bitcoin#18454. * bitcoin#18512. * bitcoin#19314. * bitcoin#19687 Keys and Addresses encoding: ---------------- * bitcoin#11372. * bitcoin#17511. * bitcoin#17721. Util: ---------------- * bitcoin#9140. * bitcoin#16577. * bitcoin#16889. * bitcoin#19593. Bench: ---------------- * bitcoin#16299. BIP155: ---------------- * bitcoin#19351. * bitcoin#19360. * bitcoin#19534. * bitcoin#19628. * bitcoin#19841. * bitcoin#19845. * bitcoin#19954. * bitcoin#19991 (pending). * bitcoin#19845. * bitcoin#20000 (pending). * bitcoin#20120. * bitcoin#20284. * bitcoin#20564. * bitcoin#21157 (pending). * bitcoin#21564 (pending). * Fully removed v2 onion addr support. * Add hardcoded seeds. * Add release-notes, changes to files.md and every needed documentation. I'm currently working on the PRs marked as "pending", this isn't over, but I'm pretty pretty close :). What a long road.. ACKs for top commit: random-zebra: utACK ecde04a Fuzzbawls: ACK ecde04a Tree-SHA512: 82c95fbda76fce63f96d8a9af7fa9a89cb1e1b302b7891e27118a6103af0be23606bf202c7332fa61908205e6b6351764e2ec23d753f1e2484028f57c2e8b51a

…mbedded NUL characters. Add tests. 7a046cd tests: Avoid using C-style NUL-terminated strings as arguments (practicalswift) fefb916 tests: Add tests to make sure lookup methods fail on std::string parameters with embedded NUL characters (practicalswift) 9574de8 net: Avoid using C-style NUL-terminated strings as arguments in the netbase interface (practicalswift) Pull request description: Don't allow resolving of `std::string`:s with embedded `NUL` characters. Avoid using C-style `NUL`-terminated strings as arguments in the `netbase` interface Add tests. The only place in where C-style `NUL`-terminated strings are actually needed is here: ```diff + if (!ValidAsCString(name)) { + return false; + } ... - int nErr = getaddrinfo(pszName, nullptr, &aiHint, &aiRes); + int nErr = getaddrinfo(name.c_str(), nullptr, &aiHint, &aiRes); if (nErr) return false; ``` Interface changes: ```diff -bool LookupHost(const char *pszName, std::vector<CNetAddr>& vIP, unsigned int nMaxSolutions, bool fAllowLookup); +bool LookupHost(const std::string& name, std::vector<CNetAddr>& vIP, unsigned int nMaxSolutions, bool fAllowLookup); -bool LookupHost(const char *pszName, CNetAddr& addr, bool fAllowLookup); +bool LookupHost(const std::string& name, CNetAddr& addr, bool fAllowLookup); -bool Lookup(const char *pszName, CService& addr, int portDefault, bool fAllowLookup); +bool Lookup(const std::string& name, CService& addr, int portDefault, bool fAllowLookup); -bool Lookup(const char *pszName, std::vector<CService>& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions); +bool Lookup(const std::string& name, std::vector<CService>& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions); -bool LookupSubNet(const char *pszName, CSubNet& subnet); +bool LookupSubNet(const std::string& strSubnet, CSubNet& subnet); -CService LookupNumeric(const char *pszName, int portDefault = 0); +CService LookupNumeric(const std::string& name, int portDefault = 0); -bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int port, const SOCKET& hSocketRet, int nTimeout, bool *outProxyConnectionFailed); +bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int port, const SOCKET& hSocketRet, int nTimeout, bool& outProxyConnectionFailed); ``` It should be noted that the `ConnectThroughProxy` change (from `bool *outProxyConnectionFailed` to `bool& outProxyConnectionFailed`) has nothing to do with `NUL` handling but I thought it was worth doing when touching this file :) ACKs for top commit: EthanHeilman: ACK 7a046cd laanwj: ACK 7a046cd Tree-SHA512: 66556e290db996917b54091acd591df221f72230f6b9f6b167b9195ee870ebef6e26f4cda2f6f54d00e1c362e1743bf56785d0de7cae854e6bf7d26f6caccaba

… fixed in bitcoin#20852

fanquake added the P2P label Dec 16, 2019

practicalswift changed the title ~~net: Don't allow resolving of std::string:s with embedded NUL characters. Add tests.~~ net: Don't allow resolving of std::string with embedded NUL characters. Add tests. Dec 17, 2019

promag reviewed Dec 22, 2019

View reviewed changes

src/test/netbase_tests.cpp Show resolved Hide resolved

practicalswift force-pushed the c_str-cleanup-netbase branch from a32d3cc to 00e90f2 Compare December 29, 2019 15:57

practicalswift force-pushed the c_str-cleanup-netbase branch 2 times, most recently from f749e5c to 10ef76f Compare December 29, 2019 20:24

promag reviewed Dec 29, 2019

View reviewed changes

src/test/netbase_tests.cpp Show resolved Hide resolved

EthanHeilman reviewed Jan 2, 2020

View reviewed changes

practicalswift added 3 commits January 8, 2020 12:35

net: Avoid using C-style NUL-terminated strings as arguments in the n…

9574de8

…etbase interface

tests: Add tests to make sure lookup methods fail on std::string para…

fefb916

…meters with embedded NUL characters

tests: Avoid using C-style NUL-terminated strings as arguments

7a046cd

practicalswift force-pushed the c_str-cleanup-netbase branch from 10ef76f to 7a046cd Compare January 8, 2020 12:36

EthanHeilman reviewed Jan 8, 2020

View reviewed changes

src/net.cpp Show resolved Hide resolved

EthanHeilman approved these changes Jan 8, 2020

View reviewed changes

fanquake requested a review from promag January 14, 2020 09:48

practicalswift requested review from jamesob and laanwj January 14, 2020 22:37

laanwj merged commit 7a046cd into bitcoin:master Jan 22, 2020

hebasto mentioned this pull request Jan 23, 2020

gui: Bilingual GUI error messages #16224

Merged

MarkLTZ mentioned this pull request Apr 4, 2020

Bitcoin PR tracking litecoinz-core/litecoinz#14

Closed

practicalswift deleted the c_str-cleanup-netbase branch April 10, 2021 19:39

kwvg pushed a commit to kwvg/dash that referenced this pull request May 22, 2021

tests: reverse tests introduced with bitcoin#13671 and bitcoin#17754

466b153

furszy mentioned this pull request Jun 7, 2021

Road to Tor v3 support (BIP155) PIVX-Project/PIVX#2411

Merged

str4d mentioned this pull request Jul 18, 2022

TorV3 Backport ZIP155: For test purposes only zcash/zcash#5366

Draft

knst added a commit to knst/dash that referenced this pull request Aug 6, 2022

Fix backport bitcoin#17754 due to missing functionality yet. Would be…

c6eeee3

… fixed in bitcoin#20852

bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

net: Don't allow resolving of std::string with embedded NUL characters. Add tests. #17754

net: Don't allow resolving of std::string with embedded NUL characters. Add tests. #17754

practicalswift commented Dec 16, 2019 •

edited

Loading

DrahtBot commented Dec 16, 2019 •

edited

Loading

elichai commented Dec 16, 2019

instagibbs commented Dec 16, 2019

laanwj commented Dec 17, 2019

promag left a comment

fanquake commented Dec 29, 2019

promag left a comment

EthanHeilman left a comment

practicalswift commented Jan 8, 2020

EthanHeilman commented Jan 8, 2020

laanwj commented Jan 22, 2020

net: Don't allow resolving of std::string with embedded NUL characters. Add tests. #17754

net: Don't allow resolving of std::string with embedded NUL characters. Add tests. #17754

Conversation

practicalswift commented Dec 16, 2019 • edited Loading

DrahtBot commented Dec 16, 2019 • edited Loading

Conflicts

elichai commented Dec 16, 2019

instagibbs commented Dec 16, 2019

laanwj commented Dec 17, 2019

promag left a comment

Choose a reason for hiding this comment

fanquake commented Dec 29, 2019

promag left a comment

Choose a reason for hiding this comment

EthanHeilman left a comment

Choose a reason for hiding this comment

practicalswift commented Jan 8, 2020

EthanHeilman commented Jan 8, 2020

laanwj commented Jan 22, 2020

practicalswift commented Dec 16, 2019 •

edited

Loading

DrahtBot commented Dec 16, 2019 •

edited

Loading