PaycoinFoundation / paycoin

Paycoin
https://paycoin.com
MIT License
42 stars 138 forks source link

Miniupnpc security vunerabilities #264

Open MitchellCash opened 8 years ago

MitchellCash commented 8 years ago

The miniupnpc codebase seems to contain vulnerabilities and Bitcoin seems to be moving towards removing the dependency completely.

Until they can remove the dependency completely their compromise for now is to at least disable it by default, to prevent UPnP vulnerabilities being a structural danger to the network.

Can @IngCr3at1on please comment if we should also be moving towards this and if, yes, should we prioritise this work to mitigate the risk as soon as possible.

Possible work required:

IngCr3at1on commented 8 years ago

Yes it is a concern, I spent all of friday trying to update our gitian dependencies but succeeded in breaking the windows builds repeatedly.

IngCr3at1on commented 8 years ago

Took another shot at trying to update the gitian descriptors to use a newer version of miniupnpc but I just get the following no matter what I do:

./build/net.o:net.cpp:(.text+0x7ce8): undefined reference to `__imp__upnpDiscover'
./build/net.o:net.cpp:(.text+0x7d20): undefined reference to `__imp__UPNP_GetValidIGD'
./build/net.o:net.cpp:(.text+0x7d54): undefined reference to `__imp__freeUPNPDevlist'
./build/net.o:net.cpp:(.text+0x7f00): undefined reference to `__imp__UPNP_AddPortMapping'
./build/net.o:net.cpp:(.text+0x800b): undefined reference to `__imp__UPNP_AddPortMapping'
./build/net.o:net.cpp:(.text+0x8022): undefined reference to `__imp__strupnperror'
./build/net.o:net.cpp:(.text+0x8068): undefined reference to `__imp__FreeUPNPUrls'
./build/net.o:net.cpp:(.text+0x8076): undefined reference to `__imp__strupnperror'
./build/net.o:net.cpp:(.text+0x80d3): undefined reference to `__imp__UPNP_GetExternalIPAddress'
./build/net.o:net.cpp:(.text+0x8186): undefined reference to `__imp__UPNP_DeletePortMapping'
./build/net.o:net.cpp:(.text+0x81a5): undefined reference to `__imp__freeUPNPDevlist'
./build/net.o:net.cpp:(.text+0x81b1): undefined reference to `__imp__FreeUPNPUrls'

The linux builds complete without issues, thought since you had windows builds working locally you might have some thoughts on this @mitchellcash

(4 hours)

IngCr3at1on commented 8 years ago

Yeah I'm making it nowhere on this (4.75 hours)

MitchellCash commented 8 years ago

This is without any testing but can you try adding DMINIUPNP_STATICLIB to your DEFINES?

IngCr3at1on commented 8 years ago

@mitchellcash doesn't look like that works either :( (2 hours)

MitchellCash commented 8 years ago

Damn! So you made sure to define -DMINIUPNP_STATICLIB instead of -DSTATICLIB?

I was certain that would solve it lol

IngCr3at1on commented 8 years ago

I added the define to the gitian descriptor during the build.

0xcircuitbreaker commented 5 years ago

was any fix found @IngCr3at1on ?

IngCr3at1on commented 5 years ago

Nope, any resolved issues were closed ;)