[geary] Drop NULs and process entire IMAP line



commit c7b3771c462deb378bb5559333149452668737f3
Author: Jim Nelson <jim yorba org>
Date:   Fri Oct 31 17:28:46 2014 -0700

    Drop NULs and process entire IMAP line
    
    Discovered while working on Turkish UTF-8 bug, technically the IMAP
    Deserializer was missing two things: (a) NUL is never allowed in an
    IMAP line, even if the string is quoted, and so it should be dropped
    rather than processed and cause potential issues, and (b)
    DataInputStream will read to EOL, potentially leaving embedded NULs in
    the line, meaning the old code would stop without processing the
    entire IMAP response.
    
    Although no server has been reported with these issues, I felt it
    important to get this right as a defensive measure.

 src/engine/imap/transport/imap-deserializer.vala |   21 +++++++++++++--------
 1 files changed, 13 insertions(+), 8 deletions(-)
---
diff --git a/src/engine/imap/transport/imap-deserializer.vala 
b/src/engine/imap/transport/imap-deserializer.vala
index 5b4f520..55e0d40 100644
--- a/src/engine/imap/transport/imap-deserializer.vala
+++ b/src/engine/imap/transport/imap-deserializer.vala
@@ -295,7 +295,7 @@ public class Geary.Imap.Deserializer : BaseObject {
             
             bytes_received(bytes_read);
             
-            push_line(line);
+            push_line(line, bytes_read);
         } catch (Error err) {
             push_error(err);
             
@@ -335,15 +335,20 @@ public class Geary.Imap.Deserializer : BaseObject {
         next_deserialize_step();
     }
     
-    // Push a line (without the CRLF!).
-    private Mode push_line(string line) {
+    // Push a line (without the CRLF!).  Because DataInputStream reads to EOL but accepts all other
+    // characters, it's possible for NULs to be embedded in the string, so the count must be passed
+    // as well (and shouldn't be -1!)
+    private Mode push_line(string line, size_t count) {
         assert(get_mode() == Mode.LINE);
         
-        int index = 0;
-        for (;;) {
-            char ch = line[index++];
-            if (ch == String.EOS)
-                break;
+        for (long ctr = 0; ctr < count; ctr++) {
+            char ch = line[ctr];
+            if (ch == String.EOS) {
+                // drop on the floor; IMAP spec does not allow NUL in lines at all, see
+                // http://tools.ietf.org/html/rfc3501#section-9 note 3.
+                // This also catches the terminating NUL.
+                continue;
+            }
             
             if (fsm.issue(Event.CHAR, &ch) == State.FAILED) {
                 deserialize_failure();


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]