Bug 192415
Summary: | suphp: double free says glibc | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Chris Jones <rollercow> | ||||||
Component: | mod_suphp | Assignee: | Andreas Thienemann <andreas> | ||||||
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||||
Severity: | medium | Docs Contact: | |||||||
Priority: | medium | ||||||||
Version: | 5 | CC: | elsmorian, extras-qa, sitsofe | ||||||
Target Milestone: | --- | ||||||||
Target Release: | --- | ||||||||
Hardware: | x86_64 | ||||||||
OS: | Linux | ||||||||
Whiteboard: | |||||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||||
Doc Text: | Story Points: | --- | |||||||
Clone Of: | Environment: | ||||||||
Last Closed: | 2006-09-08 18:23:39 UTC | Type: | --- | ||||||
Regression: | --- | Mount Type: | --- | ||||||
Documentation: | --- | CRM: | |||||||
Verified Versions: | Category: | --- | |||||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||
Cloudforms Team: | --- | Target Upstream Version: | |||||||
Embargoed: | |||||||||
Attachments: |
|
Description
Chris Jones
2006-05-19 15:44:33 UTC
Which apache version are you using? The stock FC5 apache: httpd-2.2.0-5.1.2 glibc wasn't wrong. valgrind agrees too (when running suphp from the command line where it spits out an error). Patch to come: Thx a lot. I've been trying to reproduce this and found the same error. However, I couldn't really find the reason for the double free yet. A patch would be greatly appreciated. Created attachment 131147 [details] A hard won patch for the double free error (I'm new to C++ - I just started doing it yesterday so this patch could well be wrong) The problem appears to stem from the "static initialization order fiasco" (see http://www.parashift.com/c++-faq-lite/ctors.html#faq-10.12 ). SmartPtr uses the std::map object internally but when a SmartPtr object is made in a static scope there is a chance that other object destructors will have been called before SmartPtr's destructor. Thus when SmartPtr goes to use std::map's find in its destructor there's a chance that either std::map or possibly the pairs inside the map have already been deleted causing an invalid read. glibc doesn't notice this. When the freed object is returned by the find, SmartPtr goes and tries to free the returned item again causing the double free (which glibc does notice). The change I made was to use a static pointer to an object so that SmartPtr's destructor is called when all the other regular objects' destructors are called rather than when the static classes' destructors are called (all non static object destructors always seem to be called before static objects or classes' destructors). Created attachment 131148 [details]
Updated specfile (watch out for the FC5 define at the top!)
(Side note. I'm mildly worried about how complex suphp is. If subtle bugs like
this are cropping up what other horrors might be lying in wait? I wish it were
simpler C but I guess this C++ version doesn't leak any memory...)
While I'm here, here's an updated specfile that tightens up the permissions and
ownership on the suexec binary so that they are similar to those of suexec and
less likely to be exploitable. Also adds a changelog entry and forces a define
at the top for FC5 (which probably needs to be removed). It would be nice if
things could be reworked slightly so that a plain old make works because
inbetween that define and needing to make suphp I found compiling this RPM
rather frustrating...
(In reply to comment #6) > While I'm here, here's an updated specfile that tightens up the permissions and > ownership on the suexec binary so that they are similar to those of suexec and > less likely to be exploitable. Uhm, you're changing the permissions to 4550, root, apache instead of 4755, root, root. The Owner change is actually a good idea, but why 550? that would make the binary not executable. > Also adds a changelog entry and forces a define at the top for FC5 (which > probably needs to be removed). It would be nice if things could be reworked > slightly so that a plain old make works because inbetween that define and > needing to make suphp I found compiling this RPM rather frustrating... Sorry. This is problematic as the spec should be compilable on FC3 to rawhide. What you could do however is call rpmbuild --rebuild --define "fedora 5" mod_suphp-0.6.1-2.src.rpm That way, you do not need to hardcode fedora 5 in the .spec. OK thanks for the --define (I'm just lazy and couldn't find the correct thing in
man rpmbuild). However it might be nice to make the default case the one that
compiles it correctly on the newest FC.
> Uhm, you're changing the permissions to 4550, root, apache instead of 4755,
> root, root.
> The Owner change is actually a good idea, but why 550? that would make the
> binary not executable.
Check your octal. The perms should come out as -r-sr-x--- (i.e. setuid root, and
executable by only root and apache group). Since httpd runs within the apache
group that means the number of people who can run the setuid binary has been
restricted somewhat to those who actually need to run it.
This bug and a couple of others have been fixed in the upstream version 0.6.2 |