Package: otf2bdf
Version: 3.1-4.1
Severity: normal
Tags: patch
While trying to fix an entirely different bug, I spotted this highly dubious
temporary file handling in the Debian otf2bdf package (version 3.1-4.1):
if ((tmpdir = getenv("TMPDIR")) == 0)
tmpdir = "/tmp";
sprintf(tmpfile, "%s/otf2bdf%ld", tmpdir, (long) getpid());
if ((tmp = fopen(tmpfile, "w")) == 0) {
fprintf(stderr, "%s: unable to open temporary file '%s'.\n",
prog, tmpfile);
In summary, the program opens a file in a shared temporary directory with
an entirely predictable filename based on otf2bdf's PID. This means that
if I can create files called "/tmp/otf2bdf" followed by the next few
thousand PIDs, I can stop otf2bdf working for other users.
The Debian security team has told me that Debian's /tmp hardening means
that this isn't considered a security bug, but it's still rather
unfortunate.
I've attached a patch that fixes the problem by using the standard tmpfile()
function instead. This also has the nice feature that it stops otf2bdf
from leaving temporary files behind, which it seems to do for me.
-- System Information:
Debian Release: 12.7
APT prefers stable-updates
APT policy: (500, 'stable-updates'), (500, 'stable-security'), (500, 'stable')
Architecture: amd64 (x86_64)
Foreign Architectures: i386
Kernel: Linux 6.1.0-26-amd64 (SMP w/8 CPU threads; PREEMPT)
Locale: LANG=en_GB.UTF-8, LC_CTYPE=en_GB.UTF-8 (charmap=UTF-8), LANGUAGE not set
Shell: /bin/sh linked to /usr/bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled
Versions of packages otf2bdf depends on:
ii libc6 2.36-9+deb12u8
ii libfreetype6 2.12.1+dfsg-5+deb12u3
otf2bdf recommends no packages.
otf2bdf suggests no packages.
--
Ben Harris
From 1281b4b8b8578aab5600342bfa8457649276222e Mon Sep 17 00:00:00 2001
From: Ben Harris <bj...@bjh21.me.uk>
Date: Wed, 30 Oct 2024 21:11:34 +0000
Subject: [PATCH] Fix unsafe /tmp handling
otf2bdf opened a file in /tmp with a predictable name and without taking
any precautions to ensure that it didn't already exist. This meant that
it was trivial for a user other than the one running otf2bdf to cause
trouble by creating those files ahead of time. This doesn't seem to be
usefully exploitable on a default Debian system, though.
This commit fixes the problem by using the tmpfile() function instead,
which also has the benefit of somewhat simplifying the code, since
there's no need to clean up the temporary file afterwards.
---
otf2bdf.c | 38 ++++++++------------------------------
1 file changed, 8 insertions(+), 30 deletions(-)
diff --git a/otf2bdf.c b/otf2bdf.c
index ca76fdf..74ab6fa 100644
--- a/otf2bdf.c
+++ b/otf2bdf.c
@@ -733,7 +733,6 @@ generate_font(FILE *out, char *iname, char *oname)
unsigned char *bp;
double dw;
char *xp, xlfd[BUFSIZ];
- char *tmpdir, tmpfile[BUFSIZ];
imetrics = face->size->metrics;
horizontal = FT_Get_Sfnt_Table(face, ft_sfnt_hhea);
@@ -747,12 +746,8 @@ generate_font(FILE *out, char *iname, char *oname)
* Open a temporary file to store the bitmaps in until the exact number
* of bitmaps are known.
*/
- if ((tmpdir = getenv("TMPDIR")) == 0)
- tmpdir = "/tmp";
- sprintf(tmpfile, "%s/otf2bdf%ld", tmpdir, (long) getpid());
- if ((tmp = fopen(tmpfile, "w")) == 0) {
- fprintf(stderr, "%s: unable to open temporary file '%s'.\n",
- prog, tmpfile);
+ if ((tmp = tmpfile()) == 0) {
+ fprintf(stderr, "%s: unable to open temporary file.\n", prog);
return -1;
}
@@ -931,43 +926,27 @@ generate_font(FILE *out, char *iname, char *oname)
fprintf(tmp, "ENDCHAR\n");
}
- fclose(tmp);
-
/*
- * If a write error occured, delete the temporary file and issue an error
- * message.
+ * If a write error occured, issue an error message.
*/
if (eof == EOF) {
- (void) unlink(tmpfile);
- fprintf(stderr, "%s: problem writing to temporary file '%s'.\n",
- prog, tmpfile);
+ fprintf(stderr, "%s: problem writing to temporary file.\n", prog);
return -1;
}
/*
- * If no characters were generated, just unlink the temp file and issue a
- * warning.
+ * If no characters were generated, just issue a warning.
*/
if (ng == 0) {
- (void) unlink(tmpfile);
fprintf(stderr, "%s: no glyphs generated from '%s'.\n", prog, iname);
return -1;
}
/*
- * Reopen the temporary file so it can be copied to the actual output
+ * Rewind the temporary file so it can be copied to the actual output
* file.
*/
- if ((tmp = fopen(tmpfile, "r")) == 0) {
- /*
- * Unable to open the file for read, so attempt to delete it and issue
- * an error message.
- */
- (void) unlink(tmpfile);
- fprintf(stderr, "%s: unable to open temporary file '%s' for read.\n",
- prog, tmpfile);
- return -1;
- }
+ rewind(tmp);
/*
* Calculate the average width.
@@ -1083,10 +1062,9 @@ generate_font(FILE *out, char *iname, char *oname)
}
/*
- * Close the temporary file and delete it.
+ * Close the temporary file.
*/
fclose(tmp);
- (void) unlink(tmpfile);
/*
* If an error occured when writing to the output file, issue a warning
--
2.39.5
