| Summary: | net-misc/miniupnpd _FORTIFY_SOURCE indicates presence of overflow | ||
|---|---|---|---|
| Product: | Gentoo Linux | Reporter: | Diego Elio Pettenò (RETIRED) <flameeyes> |
| Component: | Current packages | Assignee: | Bjarke Istrup Pedersen (RETIRED) <gurligebis> |
| Status: | RESOLVED FIXED | ||
| Severity: | major | CC: | hardened |
| Priority: | High | ||
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Package list: | Runtime testing required: | --- | |
| Bug Depends on: | |||
| Bug Blocks: | 259417 | ||
| Attachments: |
Build log
Patch to miniupnpd-1.5_pre20091222.ebuild to address numerous issues |
||
|
Description
Diego Elio Pettenò (RETIRED)
2010-09-20 11:35:51 UTC
Created attachment 248127 [details]
Build log
Hey :-) Why doesn't this get reported directly to upstream as portage states it should? * Please do not file a Gentoo bug and instead report the above QA * issues directly to the upstream developers of this software. * Homepage: http://miniupnp.free.fr/ I'm not that much into this problem, so I'm not totally sure on how to explain it to upstream anyway. Could I get you to take it upstream? See http://blog.flameeyes.eu/2010/09/15/is-fixing-implicit-declarations-for-me http://blog.flameeyes.eu/2010/09/14/not-all-failures-are-caused-equal http://blog.flameeyes.eu/2010/09/12/some-_fortify_source-far-fetched-warnings-are-funny http://blog.flameeyes.eu/2010/09/10/are-you-ready-for-the-next-phase … no it's not something we should be simply ignoring and wait for upstream; and I cannot bring all of them upstream; if you really can't seem to find the problem after looking into it I'll try to make some time to look into it, but there are just too many things on my plate already… (In reply to comment #2) > * Please do not file a Gentoo bug and instead report the above QA Is not this reasoning flawed as this is a bug that maybe should be considered when it is stabilization-time, and if there is no bugs open in Gentoo-bugzilla, how do you track them? (In reply to comment #2) > Why doesn't this get reported directly to upstream as portage states it should? Some upstream providers have been very reluctant to recognize or act upon bugs reported based solely on static analysis, even for cases like this where (barring a bug in the static analyzer code), the upstream code is clearly wrong. I have no idea whether this _particular_ upstream is reluctant to act. This is just a comment on why it has become standard practice for Tinderbox bugs to come to Gentoo bugzilla first. I suspect the phrasing of that message is a remnant from when it warned about code which was questionable, but not guaranteed to fail. In particular, implicit declarations are considered poor practice, but may work fine _if_ the author did everything else right. Warnings about "will always overflow destination buffer" are an indication of a real bug in every case, and can in some cases be a security issue for people who build without fortification. Diego is set up to report Tinderbox-discovered bugs to the Gentoo bugzilla in a relatively quick fashion, and there are interested Gentoo users who post proposed fixes. Once a fix has been provided in Gentoo, it can be pushed upstream by either the author of the fix or the Gentoo maintainer. This may not be as nice as having it reported directly upstream, but it is more efficient than having Diego try to track down the myriad ways in which upstream providers accept bug reports (e-mail to bugs@, e-mail to some maintainer's personal account, Bugzilla, Mantis, Trac, Google Code, etc.). Created attachment 248244 [details, diff]
Patch to miniupnpd-1.5_pre20091222.ebuild to address numerous issues
This turned out to be more complicated than expected. :) This ebuild patch fixes the following issues:
- Upstream used uuidgen, but the ebuild did not DEPEND on sys-apps/util-linux to provide uuidgen.
- User CFLAGS were ignored!
- Compilation fails if the kernel was built out-of-tree, since the ebuild inserts a reference to -I${KV_OUT_DIR}/include, but the required headers are in the kernel source tree, not the kernel object tree. Variable changed to KERNEL_DIR.
- Delete upstream call to genuuid. The rule did genuuid||uuidgen. With the dependency on util-linux, we can guarantee uuidgen is available, so do not even try to run genuuid.
- Fix the reported overflows by switching to use sizeof() to compute the available length. The compiler warned because the wrong magic constant was used, so strncpy was told a larger size than was actually true.
- Missing ||die on the calls to sed. Calls to sed the Makefile were consolidated and given an ||die.
With regard to the UUID generation, it seems a little questionable to do that in the package installation phase, since then all installations derived from the built package will share a UUID. This is the same type of problem as affected SSL certificates and led to the use of ssl-cert.eclass. I do not see any obvious security problems from allowing two miniupnpd instances to share a UUID, but I doubt clients will handle it gracefully.
I have added the patch, and bumped it to 20100921, to hit two birds with one stone. Please let me know if this fixes the problem. I cannot get my own machine to produce the fortify sources errors, even with -O2 (newest ~arch gcc and glibc) - so I don't know if they are fixed. How do I get this enabled? (I cannot seem to find any info on what might be needed that I don't have) I'm closing this, since nobody is answering, and the patch has been applied. :-) |