Bug#338115: mime-codecs: base64-decode fails to properly decode many valid base64-encoded files

[Daniel Kahn Gillmor]

Package: mime-codecs
Version: 7.19-7
Severity: grave
Tags: patch
Justification: renders package unusable


It appears that the buffering recently introduced to base64-decode has
some major flaws in it that render of all base64-encoded files that
are larger than the input buffer size (~36K) unreadable with the new
base64-decode.  This appears to be the case despite the same encoded
files decoding cleanly with both

 - sarge's version of base64-decode (7.19-4), and
 - perl -M"MIME::Base64" -ne 'print decode_base64($_)'

i'm attaching a file that appears to fail for me as "test1", but since
i don't want people who are trying to replicate this to rely on the
base64-decode in their MUA i've provided the same file here:

  http://lair.fifthhorseman.net/~dkg/src/vm/test1

Doing what should be an identity transformation on this file
highlights the problem right at the end of the file:

[EMAIL PROTECTED] test]$ <test1 base64-encode | base64-decode >test1.out
[EMAIL PROTECTED] test]$ ls -l test1 test1.out
-rw-r--r--  1 dkg dkg 26632 2005-11-08 01:16 test1
-rw-r--r--  1 dkg dkg 26632 2005-11-08 03:52 test1.out
[EMAIL PROTECTED] test]$ cmp -bl test1 test1.out
26631  32 ^Z   150 h
26632  36 ^^   170 x
[EMAIL PROTECTED] test]$ 

As you can see, the last two bytes are both left-shifted by two bits.
The input buffer is filling up a couple bytes shy of the end of the
base64-encoded version, and then the special-case handling of the
ending bytes kicks in inappropriately.  It should be kicking in only
at the end of the input stream, not at the end of the buffer.

i'm attaching a patch which appears to resolve the problem for me by
straightening out the flow of control, properly initializing some
variables, and removing the goto (considered harmful!) which doesn't
appear to be needed.

The type of failure you get from base64-decode appears to depend
exclusively on the length of the program's stdin relative to its input
buffer.  If you want to generate more test cases to see that things
fail right around 26631 bytes of un-encoded data (assuming BUFLEN is
still 72*500), try:

for N in 26630 26631 26632 26633; do
  dd if=/dev/urandom of=test$N bs=$N count=1 2>/dev/null
  <test$N base64-encode | base64-decode >test$N.out
  if (cmp -bl test$N test$N.out); then
    echo OK
  fi
done

when N=26631, you get errors from the decode that look like:

   base64-decode: base64 encoding incomplete: at least 2 bits missing

which are a result of the same buffering failure.  

I hope these diagnostics are helpful.  Thanks for maintaining this
package!  If i can provide any more examples or test cases, please let
me know.

Regards,

        --dkg

-- System Information:
Debian Release: testing/unstable
  APT prefers testing
  APT policy: (700, 'testing'), (700, 'stable'), (600, 'unstable')
Architecture: i386 (i686)
Shell:  /bin/sh linked to /bin/bash
Kernel: Linux 2.6.12-1-686
Locale: LANG=en_US, LC_CTYPE=en_US (charmap=ISO-8859-1)

Versions of packages mime-codecs depends on:
ii  libc6                         2.3.5-6    GNU C Library: Shared libraries an

mime-codecs recommends no packages.

-- no debconf information

test1
Description: Binary data

--- vm/base64-decode.c	2005-11-08 03:38:45.000000000 -0500
+++ vm.basic-fix/base64-decode.c	2005-11-08 03:37:02.000000000 -0500
@@ -4,6 +4,7 @@
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
 
 #ifdef _WIN32
 #ifndef WIN32
@@ -22,7 +23,7 @@
 main(void)
 {
     static char inalphabet[256], decoder[256];
-    int i, bits, char_count, errors = 0;
+    int i, bits = 0, char_count = 0, errors = 0;
 
 #ifdef WIN32
     _setmode( _fileno(stdout), _O_BINARY);
@@ -47,9 +48,6 @@
         len=fread(buf, sizeof(c), BUFLEN, stdin);
         if(!len) continue;
 
-cont_buffer:
-        char_count = 0;
-        bits = 0;
         while(pos<len) {
             c=buf[pos++];
             if (c == '=')
@@ -68,17 +66,20 @@
                 bits <<= 6;
             }
         }
+        fwrite(outbuf, sizeof(char), (out-outbuf), stdout);
+    }
+
         switch (char_count) {
             case 1:
                 fprintf(stderr, "base64-decode: base64 encoding incomplete: at least 2 bits missing");
                 errors++;
                 break;
             case 2:
-                *out++ = ((bits >> 10));
+                fputc((bits >> 10), stdout);
                 break;
             case 3:
-                *out++ = ((bits >> 16));
-                *out++ = (((bits >> 8) & 0xff));
+                fputc((bits >> 16), stdout);
+                fputc(((bits >> 8) & 0xff), stdout);
                 break;
             case 0:
                 break;
@@ -86,9 +87,5 @@
                 fprintf(stderr, "base64-decode: base64 encoding incomplete: at least %d bits truncated",
                         ((4 - char_count) * 6));
         }
-        if(pos<len) // did not proceed the whole thing, continue
-            goto cont_buffer;
-        fwrite(outbuf, sizeof(char), (out-outbuf), stdout);
-    }
     exit(errors ? 1 : 0);
 }