fetchmail-friends
[Top] [All Lists]

[fetchmail] [PATCH] Re: Minor security bug, how to fix?

2003-07-28 02:08:14
Quoting from Benjamin Drieu's mail on Sun, Jul 27, 2003 at 03:21:39PM +0200:
I'd like to talk about a way to fix a small security problem.  This is
not important enough to justify privacy, so let discuss all together.

...

 If the password is a substring of the username, fetchmail will
 asterisk out part of the username rather than the password.

 For example, username "test(_at_)example(_dot_)com", password "test"

 fetchmail: IMAP> A0004 LOGIN "*(_at_)example(_dot_)com" "test"

 or username "test(_at_)example(_dot_)com", password "te"

 fetchmail: IMAP> A0004 LOGIN "*st(_at_)example(_dot_)com" "te"

...

What do you think?  May I send a patch using this solution or is there
a cleaner way to achieve that?

Using a password which is a substring of username is in itself a
security problem! The only fix is to use a non-trivial password :-)

However, it is irritating which one is using trivial passwords in test
mode. This small patch will work correctly in the above cases.

===================================================================
diff -Naur fetchmail-6.2.3.orig/fetchmail.h fetchmail-6.2.3/fetchmail.h
--- fetchmail-6.2.3.orig/fetchmail.h    2003-07-28 12:39:16.000000000 +0530
+++ fetchmail-6.2.3/fetchmail.h 2003-07-28 12:39:38.000000000 +0530
@@ -391,7 +391,7 @@
 extern flag configdump;                /* dump control blocks as Python 
dictionary */
 extern char *fetchmailhost;    /* either "localhost" or an FQDN */
 extern int suppress_tags;      /* suppress tags in tagged protocols? */
-extern char shroud[PASSWORDLEN*2+1];   /* string to shroud in debug output */
+extern char shroud[PASSWORDLEN*2+3];   /* string to shroud in debug output */
 #ifdef SDPS_ENABLE
 extern char *sdps_envfrom;
 extern char *sdps_envto;
diff -Naur fetchmail-6.2.3.orig/imap.c fetchmail-6.2.3/imap.c
--- fetchmail-6.2.3.orig/imap.c 2003-07-28 12:39:16.000000000 +0530
+++ fetchmail-6.2.3/imap.c      2003-07-28 12:39:38.000000000 +0530
@@ -499,7 +499,13 @@
        imap_canonicalize(remotename, ctl->remotename, NAMELEN);
        imap_canonicalize(password, ctl->password, PASSWORDLEN);
 
-       strcpy(shroud, password);
+#ifdef HAVE_SNPRINTF
+       snprintf(shroud, sizeof (shroud), "\"%s\"", password);
+#else
+       strcpy(shroud, "\"");
+       strcat(shroud, password);
+       strcat(shroud, "\"");
+#endif
        ok = gen_transact(sock, "LOGIN \"%s\" \"%s\"", remotename, password);
        shroud[0] = '\0';
 #ifdef SSL_ENABLE
diff -Naur fetchmail-6.2.3.orig/transact.c fetchmail-6.2.3/transact.c
--- fetchmail-6.2.3.orig/transact.c     2003-07-28 12:39:16.000000000 +0530
+++ fetchmail-6.2.3/transact.c  2003-07-28 12:39:38.000000000 +0530
@@ -40,7 +40,7 @@
 
 int mytimeout;         /* value of nonreponse timeout */
 int suppress_tags;     /* emit tags? */
-char shroud[PASSWORDLEN*2+1];  /* string to shroud in debug output */
+char shroud[PASSWORDLEN*2+3];  /* string to shroud in debug output */
 struct msgblk msgblk;
 
 char tag[TAGLEN];
@@ -1422,7 +1422,7 @@
        char    *sp;
 
        sp = cp + strlen(shroud);
-       *cp = '*';
+       *cp++ = '*';
        while (*sp)
            *cp++ = *sp++;
        *cp = '\0';
===================================================================

-- 
Sunil Shetye.

<Prev in Thread] Current Thread [Next in Thread>