Skip to content

Conversation

laanwj
Copy link
Member

@laanwj laanwj commented Oct 9, 2015

Common sentiment is that the miniupnpc codebase likely contains further vulnerabilities (context: #6789).

I'd prefer to get rid of the dependency completely, but a compromise for now is to at least disable it by default, to prevent UPnP vulnerabilities being a structural danger to the network.

Also get rid of the confusing --[enable|disable]-upnp-defaultautoconf and define magic.

Edit: needs backport to 0.11 and 0.10

@laanwj laanwj added the P2P label Oct 9, 2015
@laanwj laanwj force-pushed the 2015_10_disable_upnp_default branch from e977dcd to e23ec46 Compare October 9, 2015 18:23
@laanwj laanwj mentioned this pull request Oct 9, 2015
@laanwj laanwj force-pushed the 2015_10_disable_upnp_default branch from e23ec46 to 2b54706 Compare October 9, 2015 18:27
@laanwj
Copy link
Member Author

laanwj commented Oct 9, 2015

Right, fixed

@btcdrak
Copy link
Contributor

btcdrak commented Oct 9, 2015

utACK

@luke-jr
Copy link
Member

luke-jr commented Oct 9, 2015

NACK removal of configure option; please just change the options gitian uses so people don't need to hand-patch :(

@TheBlueMatt
Copy link
Contributor

Concept ACK. I'm fine with removing the configure option, not sure we really need it. Its a very different world from when UPnP defaults were set, and I really dont think there is much need for either the GUI or bitcoind to default to UPnP on anymore.

Common sentiment is that the miniupnpc codebase likely contains further
vulnerabilities.

I'd prefer to get rid of the dependency completely, but a compromise for
now is to at least disable it by default.
@laanwj laanwj force-pushed the 2015_10_disable_upnp_default branch from 2b54706 to 21d27eb Compare October 9, 2015 19:10
@laanwj laanwj merged commit 21d27eb into bitcoin:master Oct 10, 2015
laanwj added a commit that referenced this pull request Oct 10, 2015
21d27eb net: Disable upnp by default (Wladimir J. van der Laan)
laanwj added a commit that referenced this pull request Oct 10, 2015
Common sentiment is that the miniupnpc codebase likely contains further
vulnerabilities.

I'd prefer to get rid of the dependency completely, but a compromise for
now is to at least disable it by default.

Rebased-From: 21d27eb
Github-Pull: #6795
laanwj added a commit that referenced this pull request Oct 10, 2015
Common sentiment is that the miniupnpc codebase likely contains further
vulnerabilities.

I'd prefer to get rid of the dependency completely, but a compromise for
now is to at least disable it by default.

Github-Pull: #6795
Rebased-From: 21d27eb
laanwj added a commit that referenced this pull request Oct 15, 2015
Github-Pull: #6795
Rebased-From: 21d27eb
reddink pushed a commit to reddcoin-project/reddcoin-3.10 that referenced this pull request May 27, 2020
Common sentiment is that the miniupnpc codebase likely contains further
vulnerabilities.

I'd prefer to get rid of the dependency completely, but a compromise for
now is to at least disable it by default.

Rebased-From: 21d27eb
Github-Pull: bitcoin#6795
(cherry picked from commit f2778e0)

# Conflicts:
#	contrib/gitian-descriptors/gitian-win.yml
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants