[654] in linux-security and linux-alert archive
[linux-security] Re: Vulnerabilities in mgetty+sendfax (root access by fax)
daemon@ATHENA.MIT.EDU (Gert Doering)
Tue Apr 2 18:12:19 1996
From: gert@greenie.muc.de (Gert Doering)
To: zblaxell@myrus.com (Zygo Blaxell)
Date: Tue, 2 Apr 1996 22:05:14 +0200
Cc: linux-security@tarsier.cv.nrao.edu, best-of-security@suburbia.net
In-Reply-To: <199604021811.NAA25229@shampoo.myrus> from "Zygo Blaxell" at Apr 2, 96 01:11:29 pm
Hi,
Zygo Blaxell wrote:
> (sorry about sending this to Gert twice; I found two email addresses in
> the README files)
Well, both get delivered to the same mailbox... there is not much
difference here.
[..]
> Version: all those that support FAX_NOTIFY_PROGRAM
> Platform: all
>
> Vulnerability 1: mgetty does not properly parse input from remote fax
> modem
Hmmm. Didn't know that there were *still* problems.
> Impact: if the FAX_NOTIFY_PROGRAM feature is used, and fax reception
> is allowed, anyone who can send a fax to a machine running
> mgetty can execute up to 17 bytes of shell command as root on
> that machine. This can be escalated to full-blown root access
> if the attacker has a shell account on the receiving machine.
Huh? Could you give an example? I think I know what one would do (send
something with ";", wildcards and space in it?), but I thought that it
wasn't possible to actually exploit that.
[..]
> Workaround: disable fax reception. Recompile mgetty with
> 'FAX_NOTIFY_PROGRAM' undefined in policy.h. mgetty 0.98
> is distributed with this macro defined by default.
I'll fix it in the latest 0.99...
> Exploit: Call a machine running mgetty+sendfax and send it a fax, with
> the fax modem's local ID string set to
> ';17-byte-command;
> The 17-byte-command will be executed using /bin/sh with root
> privileges.
Yes. You're right. Since the standard isn't too clear what characters
are allowed, many faxmodems allow *any* ASCII character to be sent,
including "'" and ";". *sigh*.
> Fix: in faxlib.c, function fax_wait_for(), add code to remove
> characters not in the set {alphanumeric, dot, dash, plus}
> from string 'fax_remote_id', and enforce the limit of 20
> characters.
Hmmm. Not the proper place to fix it (but the easy one). The fax ID
should be passed to the calling functions "as-is", but they should
check better before calling "system()".
> Discussion:
>
> RTFM, and you'll find the bug. Start with:
No discussion needed, you're perfectly right. I know my code. It was
a dumb thing to assume that this invocation of "system()" was tolerable
as the input data comes from a "controlled" environment...
[..]
> As far as I can tell, this specifies the allowed format of the +FTSI and
> related commands for setting and reading fax modem ID strings. Note
> that any printable ASCII data can be sent, not just numbers. This is
> even mentioned elsewhere in the mgetty manual.
Yup.
> The function fax_get_line is replaced by mdm_get_line() in mgetty
> version 0.99, but it is otherwise the same as in 0.98.
I know.
[.. many correct observations deleted for brevity...]
[..]
> Note that the Rockwell specs only allow 20 characters for fax ID.
> Certainly if the fax protocol and modem firmware allow more than 20
> characters, then there is a buffer-overrun attack possible against
> mgetty.
Hmmm. I disagree :-) -- the fax id is transmitted in a special frame
in the fax modem handshake, which has (AFAIK) not more room than 20
characters.
[..]
> Fix 1. Disallow the character ' in fax_remote_id. (Actually,
> it probably doesn't hurt to disallow anything but {alphanumerics, dot,
> dash, plus} and convert everything else to underscore or ignore it.
Yes.
> Explicitly limit the length of fax_remote_id to 20 characters.
See above...
> Fix 2. Replace the system() call with an execlp(), thus
> preventing a shell from becoming involved and also avoiding the
> potential buffer-overrun bug.
Problematic, because I have to support a few systems where a shell
script cannot be exec()ed (old SCO ODT, for example), but people
want shell scripts as FAX_NOTIFY_PROGRAM.
> Explicitly limit the length of fax_remote_id to 20 characters.
Not needed.
> Also make a note in the documentation
> about this sort of problem, as it might rear its ugly head again if
> someone uses a naive shell script as FAX_NOTIFY_PROGRAM.
Yes.
> A variation is to put the ID in an environment variable.
> This is better design but incompatible with existing practice.
It may be better design, but it isn't any little bit safer when
used in shell scripts.
> Fix 3. All of the above. Extra paranoia never hurt anyone.
Yup ;-)
> Interestingly enough, I found the following code in faxrec.c, in
> fax_get_page_data(), while looking for existing code that might plug
> this hole:
>
> > /* filter out characters that will give problems in the shell */
[..]
This is for the file names, which are more critical than the remote ID.
[..]
> However, the list is incomplete; notably absent are characters
> like ` and |.
Fixed.
> The code also doesn't modify fax_remote_id,
> which would have quite effectively plugged this hole.
Yes, this is true. I did not (and do not) want to hide information from
the user (logfile/user mail), so I do not want to modify this directly.
Hmmm, hmmm. I'll do it anyway (limit the length of the remote station
ID explicitely, and remove all quotes - the rest is left as excercise
for the shell script writer)
Patch vs. 0.99-Mar20 appended below.
(Will not work vs. 0.98, as many things look "a bit" different now)
gert
----------------------------------------------------
diff -u3 -r mgetty-0.99-orig/faxlib.c mgetty-0.99/faxlib.c
--- mgetty-0.99-orig/faxlib.c Wed Jan 3 21:32:17 1996
+++ mgetty-0.99/faxlib.c Tue Apr 2 21:53:32 1996
@@ -19,7 +19,7 @@
Modem_type modem_type = Mt_class2; /* if uninitialized, assume class2 */
-char fax_remote_id[1000]; /* remote FAX id +FTSI */
+char fax_remote_id[40]; /* remote FAX id +FTSI */
char fax_param[1000]; /* transm. parameters +FDCS */
fax_param_t fax_par_d; /* fax params detailed */
char fax_hangup = 0;
@@ -36,6 +36,19 @@
* handle all the various class 2 / class 2.0 status responses
*/
+/* copy fax station id, removing quote characters (dangerous for shell!) */
+static void fwf_copy_remote_id _P1( (id), char * id )
+{
+int w = 0;
+ while ( *id && w < sizeof(fax_remote_id)-1 )
+ {
+ if ( *id == '"' || *id == '\'' ) fax_remote_id[w++] = '_';
+ else fax_remote_id[w++] = *id;
+ id++;
+ }
+ fax_remote_id[w]=0;
+}
+
static boolean fwf_timeout = FALSE;
static RETSIGTYPE fwf_sig_alarm() /* SIGALRM handler */
@@ -89,7 +102,7 @@
strncmp( line, "+FPI:", 5 ) == 0 )
{
lprintf( L_MESG, "fax_id: '%s'", line );
- strcpy( fax_remote_id, &line[ix] );
+ fwf_copy_remote_id( &line[ix] );
}
else if ( strncmp( line, "+FDCS:", 6 ) == 0 ||
diff -u3 -r mgetty-0.99-orig/faxrec.c mgetty-0.99/faxrec.c
--- mgetty-0.99-orig/faxrec.c Sun Mar 3 22:44:56 1996
+++ mgetty-0.99/faxrec.c Tue Apr 2 22:00:46 1996
@@ -212,8 +212,8 @@
{
char c = fax_remote_id[j];
if ( c == ' ' || c == '/' || c == '\\'|| c == '&' || c == ';' ||
- c == '(' || c == ')' || c == '>' || c == '<' ||
- c == '?' || c == '*' )
+ c == '(' || c == ')' || c == '>' || c == '<' || c == '|' ||
+ c == '?' || c == '*' || c == '\''|| c == '"' || c == '`' )
{
if ( temp[i-1] != '-' ) temp[i++] = '-' ;
}
----------------------------------------------------
--
Gert Doering
Mobile communications ... right now writing from *AWAY* :-))
+49 177 2160 221 or +49 8251 4470 gert@greenie.muc.de