Bug 327854 (CVE-2007-5199) - VUL-0: CVE-2007-5199: xorg-x11-libs: off by 1 in libXfont 1.3.1
Summary: VUL-0: CVE-2007-5199: xorg-x11-libs: off by 1 in libXfont 1.3.1
Status: RESOLVED FIXED
Alias: CVE-2007-5199
Product: SUSE Security Incidents
Classification: Novell Products
Component: Incidents (show other bugs)
Version: unspecified
Hardware: Other Other
: P2 - High : Normal
Target Milestone: ---
Assignee: Security Team bot
QA Contact: Security Team bot
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-09-24 16:20 UTC by Marcus Rückert
Modified: 2017-08-02 06:27 UTC (History)
4 users (show)

See Also:
Found By: ---
Services Priority:
Business Priority:
Blocker: ---
Marketing QA Status: ---
IT Deployment: ---


Attachments
libXfont-off_by_one.diff (396 bytes, patch)
2007-09-26 09:31 UTC, Stefan Dirsch
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Marcus Rückert 2007-09-24 16:20:56 UTC
----- Forwarded message from Joerg Sonnenberger <joerg@britannica.bec.de> -----

Date: Fri, 21 Sep 2007 23:28:43 +0200
From: Joerg Sonnenberger <joerg@britannica.bec.de>
To: xorg_security@x.org
Subject: Off-by-one in libXfont-1.3.1

Hi,
the update from libXfont-1.3.0 to 1.3.1 changed catalogue.c to
NUL-terminate the buffer passed to readlink. This is not correct if
readlink writes exactly sizeof dest bytes as it writes exactly one byte
after the limit. The old code was not vulnerable as it copied to a newly
allocated buffer of the right size.

This code should be somewhat reorganised to use asprintf/snprintf and
making it a lot more readable, but that is a visual matter independent
of the actual issue.

Regards,
Joerg Sonnenberger

----- End forwarded message -----

i took joerg as original reporter into CC
Comment 1 Thomas Biege 2007-09-25 09:46:49 UTC
Jörg,
can you add the affected code snippet please.


The only occurrence I found: 10.2/xorg-x11-libs/libXfont-1.2.3.tar.bz2


Comment 2 Marcus Rückert 2007-09-25 10:13:31 UTC
include/X11/fonts/fntfil.h:#define MAXFONTFILENAMELEN  1024

in src/fontfile/catalogue.c:
[[[
static int
CatalogueRescan (FontPathElementPtr fpe)
{
[...]
    char                dest[MAXFONTFILENAMELEN];
[...]
        len = readlink(link, dest, sizeof dest);
        if (len < 0)
            continue;

        dest[len] = '\0';
[...]
}
]]]

if the filename read via readlink now is 1024 chars long you set the 1025th byte to \0.

this only affects the 10.3/STABLE package.
Comment 3 Stefan Dirsch 2007-09-25 19:54:53 UTC
Any patch available?
Comment 4 Thomas Biege 2007-09-26 03:47:18 UTC
use:
len = readlink(link, dest, sizeof(dest)-1);
Comment 5 Stefan Dirsch 2007-09-26 04:31:42 UTC
Thanks, Thomas. Yesterday I was just to tired to think about it myself. Am I allowed to fix this for 10.3/STABLE right now?
Comment 6 Thomas Biege 2007-09-26 05:44:08 UTC
Hm, I am not sure if this is public.

And I wonder wehy I didn't see the mail on the xorg-security list. I'll contact xorg-security@ directly... stay tuned.
Comment 7 Stefan Dirsch 2007-09-26 09:28:56 UTC
catalogue dir support is a new feature, which came *after* openSUSE 10.2. I'll attach against openSUSE 10.3/STABLE.
Comment 8 Stefan Dirsch 2007-09-26 09:31:56 UTC
Created attachment 174792 [details]
libXfont-off_by_one.diff

patch against openSUSE 10.3/STABLE
Comment 9 Stefan Dirsch 2007-09-26 10:40:52 UTC
BTW, this is a security fix for a feature, SUSE currently does not use at all.
It would have been an interesting feature about 10 years ago, when client side font rendering didn't exist yet ...
Comment 10 Stefan Dirsch 2007-09-28 06:51:36 UTC
commit 5bf703700ee4a5d6eae20da07cb7a29369667aef
Author: Matthieu Herrb <matthieu@bluenote.herrb.com>
Date:   Fri Sep 28 08:17:57 2007 +0200

    catalogue.c: prevent a one character overflow
    
    this occurs if readlink writes a result that's exactly the
    size of the buffer that's passed to it. Reported by
    Joerg Sonnenberger.
    
    Re

diff --git a/src/fontfile/catalogue.c b/src/fontfile/catalogue.c
index 33d4434..c0d90f8 100644
--- a/src/fontfile/catalogue.c
+++ b/src/fontfile/catalogue.c
@@ -156,7 +156,7 @@ CatalogueRescan (FontPathElementPtr fpe)
     while (entry = readdir(dir), entry != NULL)
     {
        snprintf(link, sizeof link, "%s/%s", path, entry->d_name);
-       len = readlink(link, dest, sizeof dest);
+       len = readlink(link, dest, sizeof dest - 1);
        if (len < 0)
            continue;
 
Comment 11 Stefan Dirsch 2007-10-01 20:36:21 UTC
Only 10.3 and STABLE is affected by this and I'm not aware of any configuration tool, which currently generate such catalogue lines to xorg.conf. Should we provide an update for 10.3 at all or would it be enough
to apply the patch for STABLE?
Comment 12 Thomas Biege 2007-10-02 06:40:59 UTC
Let's do it for 10.3 too it won't hurt.
Comment 13 Stefan Dirsch 2007-10-02 07:17:19 UTC
Ok. I submitted xorg-x11-libs package now for 10.3 and STABLE. Could you provide a SWAMPID?
Comment 15 Thomas Biege 2007-10-02 09:11:49 UTC
MaintenanceTracker-13588
Comment 16 Thomas Biege 2007-10-02 09:29:31 UTC
- bug is still NOT PBLIC
- requested a CVE-ID
Comment 18 Stefan Dirsch 2007-10-02 09:39:15 UTC
Thanks. Patchinfo file submitted.

--> /work/src/done/PATCHINFO/xorg-x11-libs.patchinfo
Comment 19 Jörg Sonnenberger 2007-10-02 12:53:06 UTC
(In reply to comment #11 from Stefan Dirsch)
> Only 10.3 and STABLE is affected by this and I'm not aware of any configuration
> tool, which currently generate such catalogue lines to xorg.conf.

It is my understanding that a user can trigger this using xset +fp.

Joerg
Comment 20 Stefan Dirsch 2007-10-03 08:35:12 UTC
(In reply to comment #16 from Thomas Biege)
> - bug is still NOT PBLIC
It's already in git (see comment #10), so it is just public.
Comment 21 Thomas Biege 2007-10-04 17:52:32 UTC
CVE-2007-5199
Comment 22 Thomas Biege 2007-10-08 14:35:17 UTC
released