Dan, good day.
Sun, Mar 02, 2008 at 01:48:17AM +0100, Dan Lukes wrote:
> Eygene Ryabinkin napsal/wrote, On 03/02/08 00:06:
>>> 1. Run ppp
>>> 2. type the following (or atleat some variation
of)
> ...
>
>> Yes, good catch: looks like stack-based buffer
overflow
>
>> Could you please test the following rough patch
>
> It seems you are going to cut of part of line
silently.
>
> IMHO - the line shall be rejected as invalid at all or
warning needs to be
> issued at least ...
Yes, I will add the neccessary statements. But first I want
to
verify that the exploitation path is not available anymore.
> Someone may create so long line (unintentionally), it
will not work for him
> with no hint why - it's not so polite ...
May be the buffer should even be dynamically resized -- will
look
into it.
Thanks!
--
Eygene
_______________________________________________
freebsd-security freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-secu
rity
To unsubscribe, send any mail to
"freebsd-security-unsubscribefreebsd.org"
|
Me again.
Sun, Mar 02, 2008 at 08:59:53AM +0300, Eygene Ryabinkin
wrote:
> >> Could you please test the following rough
patch
> >
> > It seems you are going to cut of part of line
silently.
> >
> > IMHO - the line shall be rejected as invalid at
all or warning needs to be
> > issued at least ...
>
> Yes, I will add the neccessary statements. But first I
want to
> verify that the exploitation path is not available
anymore.
OK, here we go: the above patch seem to fix all issues with
the
InterpretArg function and it properly warns the user that
the
expansion was failed due to the lack of the free buffer
space.
But if someone will ask me, then I am in mood to check the
rest of
the ppp's code: maybe other issues will arise. But it won't
be
very quick.
> > Someone may create so long line (unintentionally),
it will not work for him
> > with no hint why - it's not so polite ...
>
> May be the buffer should even be dynamically resized --
will look
> into it.
Did not messed with this: too long for now.
Please, test the new patch and report back in any case.
-----
>From dd38bd346faf42a528f32e0f3fb01ad1dbd95b4f Mon Sep 17
00:00:00 2001
From: Eygene Ryabinkin <rea-fbsdcodelabs.ru>
Date: Sun, 2 Mar 2008 10:26:48 +0300
Subject: [PATCH] Fix buffer overflow in the InterpretArg
code.
No destination buffer boundary checks were done, so tilde
and
environment variable expansions could lead to the buffer
overflows.
Ordinary strings were not generally affected, since the
length of
the input (uninterpreted) buffer is at most equal to the
length of
the destination (interpreted) buffer.
Signed-off-by: Eygene Ryabinkin <rea-fbsdcodelabs.ru>
---
usr.sbin/ppp/command.c | 5 ++++-
usr.sbin/ppp/systems.c | 41
++++++++++++++++++++++++++++++++++++++++-
2 files changed, 44 insertions(+), 2 deletions(-)
diff --git a/usr.sbin/ppp/command.c
b/usr.sbin/ppp/command.c
index ed4866c..5f90888 100644
--- a/usr.sbin/ppp/command.c
+++ b/usr.sbin/ppp/command.c
-1132,7
+1132,10 command_Expand_Interpret(char *buff, int nb, char
*argv[MAXARGS], int offset)
{
char buff2[LINE_LEN-offset];
- InterpretArg(buff, buff2);
+ if (InterpretArg(buff, buff2) == NULL) {
+ log_Printf(LogWARN, "Failed to expand command
'%s': too long for the destination buffern", buff);
+ return -1;
+ }
strncpy(buff, buff2, LINE_LEN - offset - 1);
buff[LINE_LEN - offset - 1] = ' ';
diff --git a/usr.sbin/ppp/systems.c
b/usr.sbin/ppp/systems.c
index 77f06a1..1025e02 100644
--- a/usr.sbin/ppp/systems.c
+++ b/usr.sbin/ppp/systems.c
-64,7
+64,14 CloseSecret(FILE *fp)
fclose(fp);
}
-/* Move string from ``from'' to ``to'', interpreting ``~''
and $.... */
+/*
+ * Move string from ``from'' to ``to'', interpreting ``~''
and $....
+ * Returns NULL is string expansion failed due to the lack
of
+ * free space.
+ *
+ * NB: destination buffer size is hardcoded, so we rely it
to be
+ * no less then LINE_LEN characters.
+ */
const char *
InterpretArg(const char *from, char *to)
{
-82,6
+89,10 InterpretArg(const char *from, char *to)
from++;
while (*from != ' ') {
+ if (to >= endto) {
+ *endto = ' ';
+ return NULL;
+ }
switch (*from) {
case '"':
instring = !instring;
-97,6
+108,10 InterpretArg(const char *from, char *to)
*to++ = '\'; /* Pass the escapes on, maybe
skipping # */
break;
}
+ if (to >= endto) {
+ *endto = ' ';
+ return NULL;
+ }
*to++ = *from++;
break;
case '$':
-127,9
+142,17 InterpretArg(const char *from, char *to)
*ptr++ = *from;
*ptr = ' ';
}
+ if (to >= endto) {
+ *endto = ' ';
+ return NULL;
+ }
if (*to == ' ')
*to++ = '$';
else if ((env = getenv(to)) != NULL) {
+ if ((int)strlen(env) >= endto - to) {
+ *endto = ' ';
+ return NULL;
+ }
strncpy(to, env, endto - to);
*endto = ' ';
to += strlen(to);
-142,13
+165,25 InterpretArg(const char *from, char *to)
if (len == 0)
pwd = getpwuid(ID0realuid());
else {
+ if (to + len >= endto) {
+ *to = ' ';
+ return NULL;
+ }
strncpy(to, from, len);
to[len] = ' ';
pwd = getpwnam(to);
}
+ if (to >= endto) {
+ *endto = ' ';
+ return NULL;
+ }
if (pwd == NULL)
*to++ = '~';
else {
+ if ((int)strlen(pwd->pw_dir) >= endto - to) {
+ *endto = ' ';
+ return NULL;
+ }
strncpy(to, pwd->pw_dir, endto - to);
*endto = ' ';
to += strlen(to);
-185,6
+220,10 DecodeCtrlCommand(char *line, char *arg)
if (!strncasecmp(line, "include", 7) &&
issep(line[7])) {
end = InterpretArg(line+8, arg);
+ if (end == NULL) {
+ log_Printf(LogWARN, "Failed to expand command
'%s': too long for the destination buffern", line);
+ return CTRL_UNKNOWN;
+ }
if (*end && *end != '#')
log_Printf(LogWARN, "usage: !include
filenamen");
else
--
1.5.3.8
-----
Thanks!
--
Eygene
_______________________________________________
freebsd-securityfreebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-secu
rity
To unsubscribe, send any mail to
"freebsd-security-unsubscribefreebsd.org"
|
