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.

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

update miniupnpc #2378

Merged
merged 1 commit into from
Sep 14, 2017
Merged

update miniupnpc #2378

merged 1 commit into from
Sep 14, 2017

Conversation

MaxXor
Copy link
Contributor

@MaxXor MaxXor commented Aug 30, 2017

Updated the miniupnpc module from miniupnp/miniupnp@587f33c

Changelog:

  • Fix CVE-2017-8798 Thanks to tin/Team OSTStrom
  • Check strlen before memcmp in XML parsing portlistingparse.c
  • Fix build under SOLARIS and CYGWIN
  • Add python 3 compatibility to IGD test

@anonimal
Copy link
Contributor

Hi @MaxXor,

JFTR: this should be a submodule like #2133 and (eventually) #2317. Lucky for us, miniupnpc uses git so it will be easy pull. Even more important: CVE or not, unless people can keep a regular/close eye on development, we should use tagged versions and not arbitrary commits in their master branch. The CVE fix is good (why haven't they bumped their version or created a release-candidate...) but what other unknown issues could be brought into the master branch since their most recent release? They are in active development, so this is why tagged versions are usually the go-to for dependency resolution.

@MaxXor
Copy link
Contributor Author

MaxXor commented Aug 31, 2017

@anonimal Yes I agree it should be a git submodule. Maybe @fluffypony could create a fork of miniupnp under the @monero-project account?
I understand your concerns when using the latest commit from the master branch. Would it be an option to just include the CVE patch?

@moneromooo-monero
Copy link
Collaborator

Adding the CVE fix only on the release-0.11 branch too would be nice, independently of what happens here.

@fluffypony
Copy link
Contributor

I've created a repo for miniupnpc, we can switch to a submodule post-merge

@fluffypony fluffypony merged commit b338dad into monero-project:master Sep 14, 2017
fluffypony added a commit that referenced this pull request Sep 14, 2017
b338dad update miniupnpc (MaxXor)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants