--- EMAIL 1: Dec 6 2010, Gynvael to PuTTY devs ---
Hey,
I've been looking through various ANSI escape code handling and I've
noticed a bug in PuTTy's source code.
VERSION INFO:
I've been reading the putty-src from dev snapshot from a few days ago.
I tested it on a dev snapshot downloaded a few minutes ago.
DETAILS:
The problem is related to handling ANSI escape code CSI (and maybe
other? didn't check) parameters.
E.g. to change the font/bg color there is a CSI code like:
\033[1;32;43m
or
\033[<<<parameter0>>>;<<<parameter1>>>;<<<parameter2>>>;...;<<<parameterN>>>m
The parameter parsing code is the following:
--code from terminal.c--
case SEEN_CSI:
term->termstate = TOPLEVEL; /* default */
if (isdigit(c)) {
if (term->esc_nargs <= ARGS_MAX) {
if (term->esc_args[term->esc_nargs - 1] == ARG_DEFAULT)
term->esc_args[term->esc_nargs - 1] = 0;
term->esc_args[term->esc_nargs - 1] =
10 * term->esc_args[term->esc_nargs - 1] + c - '0';
}
term->termstate = SEEN_CSI;
} else if (c == ';') {
if (++term->esc_nargs <= ARGS_MAX)
term->esc_args[term->esc_nargs - 1] = ARG_DEFAULT;
--end of code--
Declarations needed to understand the problem:
int esc_args[ARGS_MAX];
int esc_nargs;
OK, so, with each semicolon ';' esc_nargs is FIRST incremented, then a
check is made.
Let's assume now, that there is over INT_MAX semicolons, so that
term->esc_nargs will get to INT_MAX and do ++ again, ending up with
INT_MIN.
INT_MIN is obviously <= ARGS_MAX, so that condition will be met in both cases.
Now, for array writting (I'll skip the -1 thing, since it doesn't
present any difference):
esc_args[esc_nargs] = something will get translated to (in pseudo code)
mov [esc_args + esc_nargs * sizeof(int)], something
Let's assume esc_nargs is INT_MIN, and sizeof(int) == 4:
mov [esc_args + INT_MIN * 4], something
Since INT_MIN is 0x80000000, INT_MIN * 4 or << 2 is 0x00000000, so we get:
mov [esc_args + 0], something
Of course, (INT_MIN + 1) will end up being esc_args+4 (in bytes), so
same as esc_args[1], (INT_MIN +2) will be esc_args[2], etc...
So, anything that is between &term->esc_args[-1] and still in the
continuous range of allocated pages on heap:
1. will be overwritten with ARG_DEFAULT
2. can be overwritten with any arbitrary variable stated as a decimal
value of an unsigned int (since the string -> int conversion of
parameters)
MISC:
A remote code execution might be possible in anything worthy of
overwriting is on the the heap after esc_args. I didn't check this
yet, so this is a "maybe".
To actually exploit this issue over 2GB of semicolons must be
transmitted. That takes some time (a few minutes), even on LAN, but
still is possible.
FIX:
Execute ++ after checking <= ARGS_MAX.
DISCLOSURE POLICY:
I'll probably publish information about this after the fix is in place
(or earlier if you are not interested in fixing this issue or if it
takes an excessive amount of time).
Even if I succeed in making a PoC code execution exploit, it will not
get published.--- EMAIL 2: Dec 7 2010, Gynvael to PuTTY devs ---
Hey!
WOW that was fast! Thanks for the fix, looks good to me :)
Btw, I've checked in details the bug yesterday, and seems it was not
exploitable into code execution the way I've described.
The reason was that at some point (around esc_nargs == INT_MIN + 32 or
+ 33) it would overwrite esc_nargs in memory (which is the first
element after the array) with ARG_DEFAULT which is 0, so the whole
process would restart.
However, there was still one chance of code execution: the handling of
\033[50;<params>"p which changes the id_string with a loop like this
(pseudo code):
for(i = 1; i <= esc_nargs; i++)
strcat(id_string, something);
since esc_nargs could be any value, this overflowed id_string at some point.
To make things worse, there is a resize_fn function pointer not much
later on the heap.
But, the attacker would have real trouble controling what would
overwrite resize_fn, so a code_exec would be at least tricky.
Anyway, your fix makes sure this won't happen :)
Cheers,