From:
[email protected]
Package: termpkg
Version: 3.3-1
Severity: grave
Hi,
these bugs still apply. Not tagging +patch since I haven't really
tested them.
Cheers.
Max
--
308E81E7B97963BCA0E6ED889D5BD511B7CDA2DC
----- Forwarded message from Max Vozeler <
[email protected]> -----
From: Max Vozeler <
[email protected]>
To:
[email protected]
Cc:
[email protected]
Subject: termpkg vulnerabilites
Date: Fri, 30 Jul 2004 16:00:18 +0200
User-Agent: Mutt/1.5.6+20040523i
Hi Oliver,
[ CCing Security Team for vulns in the testing version ]
there are a few bugs in termpkg/termnetd that can introduce security
problems and potentially lead to remote root compromise. In Debian only
testing and unstable are affected. I looked at version 3.3-1.
Feel free to forward this info as you like. Unless you want me to delay disclosure to the BTS, or the fixed package has already entered sarge by
then, I'll file +security bugs for these vulns in 10 days or so.
In short:
o buffer overflow in getOption() reading telnet suboption from the
client; reachable from termnetd.
o buffer overflow in tnlSsetSubOption() and doSubOption(); A string
of up to 256 bytes is strcpy()ed into a 128-bytes char array. The
code is also reachable from the network in termnetd.
o buffer overflow in commands-handling on the control connection port,
if enabled in termnetd (Not by default AFAICS). The bug repeats in
other parts of the control connection functions.
o buffer overflow in parseRE(), also reachable from the control
connection port.
getOption() overflow
--------------------
termpkg-3.3/libtn/tnlSubOptions.c:
30 static int option, optionCnt;
31 STatic char *pOption, optionBuffer[256];
32 static int dbg = 0;
33
..
72 /*
73 ** This function stores the character passed into the
74 ** suboption buffer, then increments the pointer in
75 ** anticipation of the next character.
76 */
77 static void getOption(int ch)
78 {
79 if (strlen(optionBuffer) < sizeof(optionBuffer) - 1)
80 {
81 *(pOption++) = ch;
82 *pOption = '\0';
83 }
84 }
85
The strlen() call here tries to catch possible overflows, but doesn't
take into account that '\0' is among the values that can be written
using getOption(). This can effectively circumvent that bounds check
and cause writes to memory past the end of optionBuffer.
There is a FILE pointer next to optionBuffer on my local i386 local
debug build that gets overwritten. If an attacker was to change the
pointer to a fake but valid FILE structure, this may enable him/her
to execute arbitrary code as root.
Would it be possible to use pointer arithmetic instead of checking
the size with strlen()? I'm thinking something like the attached termpkg-getoption-bof.diff
tnlSetSubOption() and doSubOption() overflows ---------------------------------------------
termpkg-3.3/libtn/tnlVars.c:
47 int Options[256]; /* Option table */
48 char SubOptions[256][128]; /* Sub Option storage */
49 char *pSubOptions[256]; /* Sub Option Pointers */
termpkg-3.3/libtn/tnlSubOptions.c:
111 ** This function stores the data gathered from the socket in
optionBuffer
112 ** for the option specified by the variable option into ...
..
118 static void doSubOption(int ch)
119 {
..
124 strcpy(pSubOptions[option], optionBuffer);
optionBuffer can AFAICS be 255 bytes, whereas I think the space pointed
to by pSubOptions[option] is an element of the SubOptions array and only
128 bytes. If "option" has a high or low enough value, this could make
it overflow past SubOptions.
The bug in tnlSetSubOption() is very similar. I would suggest to replace
both strcpy calls with bounds checking strncpy(), see attached file sredird-suboptarg-bof.diff
control cmd overflow
--------------------
termpkg-3.3/termnetd/tndControl.c:
235 void enable_cmd(char *arg1, char *arg2, ControlStruct *pEntry)
236 {
237 portConf *pConf;
238 char *cp, tbuf[1024];
239 int cnt;
240 int devc = 0;
241
242 strcpy(tbuf, arg1);
243 for (cp = tbuf; *cp; cp++)
244 *cp = toupper(*cp);
245 if (strcmp(tbuf, "DEVICE") == 0)
246 {
247 strcpy(arg1, arg2);
248 devc = 1;
249 }
Also affected: commands
show DEVICE <...>
allow DEVICE <...>
disconnect DEVICE <...>
deny DEVICE <...>
disable DEVICE <...>
Quick demonstration:
# gdb --args termnetd -s 7778 -v -n
Starting program: /usr/sbin/termnetd -s 7779 -v -n
termnetd[18701]: openSockets():getting host entry for the control port 7779
termnetd[18701]: openSockets():Control Port = 7779
termnetd[18701]: openSockets():Listening on Control Port
termnetd[18701]: Ready to Accept Connections
termnetd[18701]: socketSlect():Adding control port!
termnetd[18701]: socketSlect():Have Control Opened!
termnetd[18701]: socketSlect():Socket accepted, port 10!
termnetd[18701]: socketSlect():ControlDataSock = -1!
termnetd[18701]: socketSlect():maxFD = 11!
$ telnet localhost 7778
show DEVICE BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB
...
Program received signal SIGSEGV, Segmentation fault.
0x42424242 in ?? ()
(gdb) q
To fix this maybe the arg1 pointer could just be changed instead of
copying the string? I haven't tested whether this works, but have a
look at the attached sredird-control-bof.diff for what I mean. The
alternative would be to make them fixed-size strncpy()s.
parseRE() overflow
------------------
termnetd/tndAdmin.c:
949 char *getStr(char *pBuf, const char *cp)
950 {
..
957 while (*cp)
958 {
..
960 if (!escape)
961 {
..
986 if (*cp == '\t')
987 {
988 ch = ' ';
989 }
990 else
991 ch = *cp;
..
993 *(pBuf++) = ch;
994 *pBuf = '\0';
995 cp++;
1012 char *parseRE(ControlStruct *p, const char *pStr)
1013 {
1014 int x, err;
1015 char tmpBuf[20], *cp, *cp1, *rv;
1016 char portStr[80], devcStr[80], errBuf[80];
1017
[continued in next message]
--- SoupGate-Win32 v1.05
* Origin: you cannot sedate... all the things you hate (1:229/2)