A buffer overrun is possible when the size of the stack buffer is exactly the same as the actual stack size. You can see that it is possible to write two results in this case. Patch attached. --- getretmips.c.orig 2004-06-16 12:01:25.000000000 +0100 +++ getretmips.c 2004-06-16 12:03:54.000000000 +0100 @@ -180,12 +180,15 @@ } ra = (unsigned long *) sp[rc->raOffset>>2]; sp += rc->spAdjust >> 2; - *results++ = ((unsigned long) ra) - 8; if (ra[-2] == mainCall) { *results++ = 0; break; } + else + { + *results++ = ((unsigned long) ra) - 8; + } max--; } }
Created attachment 375 [details] [review] [FIXED_X11R68x] patch to fix buffer overrun
What exactly are the results of such an over-run? Arbitrary code execution? Achieving root status? Dragging the system down?
I'm told "When using the memleak allocator, you could potentially write an extra (possibly user-controlled) dword to the stack." So any final word on this bug here? Can anybody else confirm or debunk this one? If confirmed will Xorg be patching this one and doing a 6.8.2 security release?
i can't confirm this, though from the patch it certainly looks obvious. if confirmed, we'd certainly patch it, but i doubt we'd do a 6.8.2 for it. the memleak allocator is only included if UseMemLeak is defined in host.def, and no platform that i know of ships that in any configuration. however i'd definitely apply it both to head and the 6.8 stable branch so that, if a 6.8.2 were released, this would be in it. bumping severity anyway since it's a security bug.
Comment on attachment 375 [details] [review] [FIXED_X11R68x] patch to fix buffer overrun nominating for 6.8.2. this is a minor security bug, in code that no vendor ships in any configuration. i would like review by someone with stronger MIPS experience, but from casual inspection it looks correct.
Comment on attachment 375 [details] [review] [FIXED_X11R68x] patch to fix buffer overrun Approved for the X11R6.8.x branch in the 2004-11-17 release-wranglers phone call. Please don't commit it yourself, I'll handle that once the CVS service is available again.
Created attachment 1534 [details] [review] Patch including Changelog comment for 2004-12-12-trunk
Comment on attachment 1534 [details] [review] Patch including Changelog comment for 2004-12-12-trunk Patch checked-in into Xorg trunk: /cvs/xorg/xc/ChangeLog,v <-- ChangeLog new revision: 1.604; previous revision: 1.603 cvs commit: Using deprecated info format strings. Convert your scripts to use the new argument format and remove '1's from your info file format strings. /cvs/xorg/xc/util/memleak/getretmips.c,v <-- getretmips.c new revision: 1.2; previous revision: 1.1 cvs commit: Using deprecated info format strings. Convert your scripts to use the new argument format and remove '1's from your info file format strings. Mailing the commit message to xorg-commit@lists.freedesktop.org...
Comment on attachment 375 [details] [review] [FIXED_X11R68x] patch to fix buffer overrun Patch checked-in into X11R6.8.x stable branch: /cvs/xorg/xc/ChangeLog,v <-- ChangeLog new revision: 1.365.2.36; previous revision: 1.365.2.35 cvs commit: Using deprecated info format strings. Convert your scripts to use the new argument format and remove '1's from your info file format strings. /cvs/xorg/xc/util/memleak/getretmips.c,v <-- getretmips.c new revision: 1.1.1.1.6.1; previous revision: 1.1.1.1 cvs commit: Using deprecated info format strings. Convert your scripts to use the new argument format and remove '1's from your info file format strings. Mailing the commit message to xorg-commit@lists.freedesktop.org...
Fix checked-in into Xorg trunk and X11R6.8.x stable branch... ... marking bug as FIXED.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.