Summary: | sys-apps/ed produces incorrect output (bug 66400 redux) | ||
---|---|---|---|
Product: | Gentoo Linux | Reporter: | Robin Johnson <robbat2> |
Component: | [OLD] Core system | Assignee: | Gentoo's Team for Core System packages <base-system> |
Status: | RESOLVED FIXED | ||
Severity: | critical | CC: | security, ulm |
Priority: | High | ||
Version: | unspecified | ||
Hardware: | All | ||
OS: | Linux | ||
URL: | http://savannah.gnu.org/patch/index.php?func=detailitem&item_id=3628 | ||
Whiteboard: | |||
Package list: | Runtime testing required: | --- | |
Bug Depends on: | |||
Bug Blocks: | 66400 | ||
Attachments: |
edtest.cpp
dotest.sh |
Description
Robin Johnson
2004-12-08 18:25:29 UTC
Created attachment 45580 [details]
edtest.cpp
Source code for testing binary.
Created attachment 45581 [details]
dotest.sh
Script file to run test case and prove the error.
Here is the patch (from LFS) that was applied by base-system to fix the vulnerability : ============================================================ --- ed-0.2/buf.c Sat Nov 19 04:37:59 1994 +++ ed-0.2-2/buf.c Tue May 28 18:38:23 2002 @@ -200,13 +200,13 @@ int open_sbuf () { - char *mktemp (); - int u; + int u, sfd; isbinary = newline_added = 0; u = umask(077); strcpy (sfn, "/tmp/ed.XXXXXX"); - if (mktemp (sfn) == NULL || (sfp = fopen (sfn, "w+")) == NULL) + sfd = mkstemp(sfn); + if ((sfd == -1) || (sfp = fopen (sfn, "w+")) == NULL) { fprintf (stderr, "%s: %s\n", sfn, strerror (errno)); sprintf (errmsg, "Cannot open temp file"); ============================================================= I fail to see where it introduces a regression, since apparently it just switched a mktemp for a mkstemp. Maybe someone more literate on those issues will find where it fails... The error in your patch is that mkstemp returns a file descriptor for an OPEN file already (eg it runs fopen on the file). so: + sfd = mkstemp(sfn); + if ((sfd == -1) || (sfp = fopen (sfn, "w+")) == NULL) if mkstemp returns a good value, and then "sfp = fopen (sfn, "w+")" is run, opening the file again. - if ((sfd == -1) || (sfp = fopen (sfn, "w+")) == NULL) + if (sfd == -1) so this patch will fix it ? This wouldn't work, since not a file descriptor, but a FILE pointer is needed. However, I believe the patch from LFS is incorrect. Not fopen should be called, but fdopen on the descriptor: - if ((sfd == -1) || (sfp = fopen (sfn, "w+")) == NULL) + if ((sfd == -1) || (sfp = fdopen (sfd, "w+")) == NULL) true ... robbat2, that patch looks much more sane, does that resolve this for you ? Ok, your patch change there to use fdopen fixes the problem. I've put it in the tree as r5, but arches need to stabilize. The changed patch should probably be sent upstream. marked stable ;) |