| Summary: | app-emulation/x48 _FORTIFY_SOURCE indicates presence of overflow | ||
|---|---|---|---|
| Product: | Gentoo Linux | Reporter: | Diego Elio Pettenò (RETIRED) <flameeyes> |
| Component: | Current packages | Assignee: | No maintainer - Look at https://wiki.gentoo.org/wiki/Project:Proxy_Maintainers if you want to take care of it <maintainer-needed> |
| Status: | RESOLVED FIXED | ||
| Severity: | major | CC: | hardened, signals |
| 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 fix implicit declaration of exit Patch to fix buffer overflow Ebuild to apply the above patches diff -u of the imlicit declaration of exit patch diff -u of the overflow patch |
||
|
Description
Diego Elio Pettenò (RETIRED)
2010-09-12 18:44:37 UTC
Created attachment 247005 [details]
Build log
Created attachment 260928 [details, diff]
Patch to fix implicit declaration of exit
Created attachment 260929 [details, diff]
Patch to fix buffer overflow
Created attachment 260931 [details]
Ebuild to apply the above patches
This is my first attempt at patching something in Gentoo, so go easy on me if it's not up to standards. I've fixed the buffer overflow in debugger.c and the lack of a stdlib.h include in errors.c. Attached are the patches and a modified ebuild to apply them.
Thanks for the patches Kevin, but could you please re-attach them in unified format (diff -u)? That's the format we usually want to use on patches. Created attachment 260933 [details, diff]
diff -u of the imlicit declaration of exit patch
Created attachment 260934 [details, diff]
diff -u of the overflow patch
No problem. Wasn't quite sure how to create the patches, so I did my best. I will use 'diff -u' from now on.
Uploaded an ebuild for app-emulation/x48-0.6.3 with the buffer overflow patch in Bug #353080 Thanks Kevin for the updated patches. Now, the off-by-one patch tells me that the original author isn't really used to deal with C-style string functions; what it is trying to implement is something similar to cl = strdup( old_line == NULL ? "(null)" : old_line ); but it's done in a very clumsy way; the fortified source warning happen only on the second branch because there the string is constant and the compiler knows the sizes, but the same problem happens for old_line: it leaves no space for the final NULL termination and will abort just the same at runtime. I'd suggest changing the whole if statement and its branch with what I wrote above and verify whether the code uses the same stuff somewhere else. (In reply to comment #9) > I'd suggest changing the whole if statement and its branch with what I wrote > above and verify whether the code uses the same stuff somewhere else. > Yes, that's a good idea. I have been running under the assumption that I did not want to modify any code too extensively, as these are just patches for the Gentoo build. But you have a good point, if there's going to be a run-time error it should be fixed too. There are a handful of similar constructs in the code. I'll clean them up and get a new patch up ASAP. I reported the string constant problem upstream, and will amend the bug report to include all of the similar problems. I've fixed the overflow errors due to the use of strcpy+malloc+strlen and uploaded a patch for app-emulation/x48-0.6.3 in bug #353080. If the patches are acceptable to you Diego, please commit them to the tree and close these bugs. Fixed in 0.6.3, thanks Kevin! |