Net::IMAP: Denial of Service via incomplete raw argument validation

@@ -218,7 +218,7 @@ def send_data(imap, tag) = data.each do _1.send_data(imap, tag) end
 
       def validate
         return unless data.last in RawText(data: text)
-        if text.rindex(/~?\{[1-9]\d*\+?\}\z/n)
+        if text.rindex(/\{\d+\+?\}\z/n)
           raise DataFormatError, "RawData cannot end with literal continuation"
         end
       end

@@ -480,6 +480,13 @@ class RawDataTest < CommandDataTest
       assert_raise(DataFormatError) do RawData.new(data: "~literal+ ~{123+}") end
       raw = RawData.new(data: " {123} ")
       assert_equal [RawText[" {123} "]], raw.data
+
+      assert_raise(DataFormatError) do RawData.new(data: "literal {0}") end
+      assert_raise(DataFormatError) do RawData.new(data: "literal+ {0+}") end
+      assert_raise(DataFormatError) do RawData.new(data: "~literal ~{0}") end
+      assert_raise(DataFormatError) do RawData.new(data: "~literal+ ~{0+}") end
+      raw = RawData.new(data: " {0} ")
+      assert_equal [RawText[" {0} "]], raw.data
     end
 
     data(

@@ -2209,6 +2209,7 @@ def uid_expunge(uid_set)
     # provided as an array or a string.
     # See {"Argument translation"}[rdoc-ref:#search@Argument+translation]
     # and {"Search criteria"}[rdoc-ref:#search@Search+criteria], below.
+    # <em>Please note</em> the warning for when +criteria+ is a String.
     #
     # +return+ options control what kind of information is returned about
     # messages matching the search +criteria+.  Specifying +return+ should force
@@ -2619,7 +2620,8 @@ def search(...)
     # backward compatibility) but adds SearchResult#modseq when the +CONDSTORE+
     # capability has been enabled.
     #
-    # See #search for documentation of parameters.
+    # See #search for documentation of parameters.  <em>Please note</em> the
+    # warning for when +criteria+ is a String.
     #
     # ==== Capabilities
     #
@@ -2705,7 +2707,8 @@ def fetch(...)
     # {SequenceSet[...]}[rdoc-ref:SequenceSet@Creating+sequence+sets].
     # (For message sequence numbers, use #fetch instead.)
     #
-    # +attr+ behaves the same as with #fetch.
+    # +attr+ behaves the same as with #fetch.  <em>Please note</em> the #fetch
+    # warning on the +attr+ argument.
     # >>>
     #   *Note:* Servers _MUST_ implicitly include the +UID+ message data item as
     #   part of any +FETCH+ response caused by a +UID+ command, regardless of
@@ -2917,8 +2920,10 @@ def uid_move(set, mailbox)
 
     # Sends a {SORT command [RFC5256 §3]}[https://www.rfc-editor.org/rfc/rfc5256#section-3]
     # to search a mailbox for messages that match +search_keys+ and return an
-    # array of message sequence numbers, sorted by +sort_keys+.  +search_keys+
-    # are interpreted the same as for #search.
+    # array of message sequence numbers, sorted by +sort_keys+.
+    #
+    # +search_keys+ are interpreted the same as the +criteria+ argument for
+    # #search.  <em>Please note</em> the #search warning for String +criteria+.
     #
     #--
     # TODO: describe +sort_keys+
@@ -2943,8 +2948,10 @@ def sort(sort_keys, search_keys, charset)
 
     # Sends a {UID SORT command [RFC5256 §3]}[https://www.rfc-editor.org/rfc/rfc5256#section-3]
     # to search a mailbox for messages that match +search_keys+ and return an
-    # array of unique identifiers, sorted by +sort_keys+.  +search_keys+ are
-    # interpreted the same as for #search.
+    # array of unique identifiers, sorted by +sort_keys+.
+    #
+    # +search_keys+ are interpreted the same as the +criteria+ argument for
+    # #search.  <em>Please note</em> the #search warning for String +criteria+.
     #
     # Related: #sort, #search, #uid_search, #thread, #uid_thread
     #
@@ -2958,8 +2965,10 @@ def uid_sort(sort_keys, search_keys, charset)
 
     # Sends a {THREAD command [RFC5256 §3]}[https://www.rfc-editor.org/rfc/rfc5256#section-3]
     # to search a mailbox and return message sequence numbers in threaded
-    # format, as a ThreadMember tree.  +search_keys+ are interpreted the same as
-    # for #search.
+    # format, as a ThreadMember tree.
+    #
+    # +search_keys+ are interpreted the same as the +criteria+ argument for
+    # #search.  <em>Please note</em> the #search warning for String +criteria+.
     #
     # The supported algorithms are:
     #
@@ -2985,6 +2994,9 @@ def thread(algorithm, search_keys, charset)
     # Similar to #thread, but returns unique identifiers instead of
     # message sequence numbers.
     #
+    # +search_keys+ are interpreted the same as the +criteria+ argument for
+    # #search.  <em>Please note</em> the #search warning for String +criteria+.
+    #
     # Related: #thread, #search, #uid_search, #sort, #uid_sort
     #
     # ==== Capabilities

@@ -150,8 +150,8 @@ def validate
       end
     end
 
-    # Represents IMAP +text+ data, which covers everything in the IMAP grammar,
-    # except for +literal+, +literal8+, and the concluding +CRLF+.
+    # Represents IMAP +text+ or +quoted+ data, which share the same
+    # validations of decoded #data, and differ only in how they are formatted.
     #
     # +data+ may contain any 7-bit ASCII character except +NULL+, +CR+, or +LF+.
     # Any multibyte +UTF-8+ character is also allowed when the connection
@@ -164,7 +164,7 @@ def validate
     #
     # The string's bytes must be valid ASCII or valid UTF-8.  The string's
     # reported encoding is ignored, but the string is _not_ transcoded.
-    class RawText < CommandData # :nodoc:
+    class ValidNonLiteralData < CommandData
       def initialize(data:)
         data = String(data.to_str)
         unless data.encoding in Encoding::ASCII | Encoding::UTF_8
@@ -189,7 +189,17 @@ def validate
 
       def ascii_only? = data.ascii_only?
 
-      def send_data(imap, tag) = imap.__send__(:put_string, data)
+      def send_data(imap, tag = nil) = imap.__send__(:put_string, formatted)
+    end
+
+    # Represents IMAP +text+ data, which covers everything in the IMAP grammar,
+    # except for +literal+, +literal8+, and the concluding +CRLF+.
+    #
+    # NOTE: The current implementation does not verify that the connection
+    # supports UTF-8.  Future versions may validate this.
+    class RawText < ValidNonLiteralData # :nodoc:
+      # raw: no formatting necessary
+      alias formatted data
     end
 
     class RawData < CommandData # :nodoc:
@@ -268,10 +278,13 @@ def send_data(imap, tag)
       end
     end
 
-    class QuotedString < CommandData # :nodoc:
-      def send_data(imap, tag)
-        imap.__send__(:send_quoted_string, data)
-      end
+    # Represents a IMAP +quoted+ string, which can encode any valid ASCII or
+    # UTF-8 string, unless it contains any +CR+, +LF+, or +NULL+ bytes.
+    #
+    # NOTE: The current implementation does not verify that the connection
+    # supports UTF-8.  Future versions may validate this.
+    class QuotedString < ValidNonLiteralData # :nodoc:
+      def formatted = %("#{data.gsub(/["\\]/, "\\\\\\&")}")
     end
 
     class Literal < Data.define(:data, :non_sync) # :nodoc:

@@ -8,6 +8,7 @@ class CommandDataTest < Net::IMAP::TestCase
 
   Atom = Net::IMAP::Atom
   Flag = Net::IMAP::Flag
+  QuotedString = Net::IMAP::QuotedString
   Literal = Net::IMAP::Literal
   Literal8 = Net::IMAP::Literal8
   RawText = Net::IMAP::RawText
@@ -155,6 +156,83 @@ def send_data(*data, tag: TAG)
     ], imap.output
   end
 
+  class QuotedStringTest < CommandDataTest
+    test "quotes ASCII strings (no specials)" do
+      assert_equal '"INBOX"', QuotedString["INBOX"].formatted
+      imap.send_data(
+        QuotedString["INBOX"],
+        QuotedString["etc"]
+      )
+      assert_equal [
+        Output.put_string('"INBOX"'),
+        Output.put_string('"etc"'),
+      ], imap.output
+      imap.clear
+    end
+
+    test "quotes ASCII strings (atom specials)" do
+      [
+        "  with  spaces  in  string  ",
+        "with_parens()",
+        "with_list_wildcards*",
+        "with_list_wildcards%",
+        "with_resp_special]",
+        "with\x7fcontrol_char",
+        %{(){}[]%*'},
+      ].each do |string|
+        imap.send_data QuotedString[string]
+      end
+      assert_equal [
+        Output.put_string('"  with  spaces  in  string  "'),
+        Output.put_string('"with_parens()"'),
+        Output.put_string('"with_list_wildcards*"'),
+        Output.put_string('"with_list_wildcards%"'),
+        Output.put_string('"with_resp_special]"'),
+        Output.put_string(%{"with\x7fcontrol_char"}),
+        Output.put_string(%Q{"(){}[]%*'"}),
+      ], imap.output
+    end
+
+    test "escapes quoted specials" do
+      [
+        '"with" "quoted" "specials"',
+        "\\with\\quoted\\specials\\",
+        %{(){}[]%*"'\\},
+      ].each do |string|
+        imap.send_data QuotedString[string]
+      end
+      assert_equal [
+        Output.put_string('"\"with\" \"quoted\" \"specials\""'),
+        Output.put_string('"\\\\with\\\\quoted\\\\specials\\\\"'),
+        Output.put_string(%q{"(){}[]%*\"'\\\\"}),
+      ], imap.output
+    end
+
+    test "ASCII compatible string with another encodings" do
+      imap.send_data QuotedString.new("foo bar".encode("cp1252"))
+      assert_equal [
+        Output.put_string('"foo bar"'),
+      ], imap.output
+    end
+
+    test "allows ASCII control chars" do
+      text = QuotedString.new("beep\b beep\b escape!\e delete this:\x1f")
+      imap.send_data text
+      assert_equal [
+        Output.put_string(%{"beep\b beep\b escape!\e delete this:\x1f"}),
+      ], imap.output
+    end
+
+    test "quotes valid UTF-8 multibyte chars" do
+      imap.send_data QuotedString.new("föó bär")
+      imap.send_data QuotedString.new("ほげ ふが ぴよ")
+      assert_equal [
+        Output.put_string('"föó bär"'),
+        Output.put_string('"ほげ ふが ぴよ"'),
+      ], imap.output
+    end
+  end
+
   class RawTextTest < CommandDataTest
     test "allows ASCII strings with no specials" do
       imap.send_data(
@@ -229,14 +307,20 @@ class RawTextTest < CommandDataTest
         Output.put_string("ほげ ふが ぴよ"),
       ], imap.output
     end
+  end
 
+  SharedValidNonLiteralDataTests = ->(data_type) do
     data(
       "NULL" => ["with \0 NULL", /NULL\b.+\bbyte/i],
       "CR"   => ["with \r CR",   /CR\b.+\bbyte/i],
       "LF"   => ["with \n LF",   /LF\b.+\bbyte/i],
     )
     test "invalid ASCII byte" do |(text, error_message)|
-      try_multiple_encodings(error_message, text)
+      with_multiple_encodings(text) do |encoded|
+        assert_raise_with_message(DataFormatError, error_message) do
+          data_type[encoded]
+        end
+      end
     end
 
     # See Table 3-7, Well-Formed UTF-8 Byte Sequences, in The Unicode Standard:
@@ -253,7 +337,11 @@ class RawTextTest < CommandDataTest
       "windows-1252"               => "åêïõü".encode("windows-1252"),
     )
     test "invalid UTF-8" do |text|
-      try_multiple_encodings(/invalid UTF-8/i, text)
+      with_multiple_encodings(text) do |encoded|
+        assert_raise_with_message(DataFormatError, /invalid UTF-8/i) do
+          data_type[encoded]
+        end
+      end
     end
 
     def with_multiple_encodings(data)
@@ -262,15 +350,9 @@ def with_multiple_encodings(data)
       yield data.dup.force_encoding("UTF-8")
       yield data.dup.force_encoding("cp1252")
     end
-
-    def try_multiple_encodings(error_message, data)
-      with_multiple_encodings(data) do |encoded|
-        assert_raise_with_message(DataFormatError, error_message) do
-          RawText[encoded]
-        end
-      end
-    end
   end
+  QuotedStringTest.class_exec QuotedString, &SharedValidNonLiteralDataTests
+  RawTextTest     .class_exec RawText,      &SharedValidNonLiteralDataTests
 
   class RawDataTest < CommandDataTest
     test "simple raw text" do

@@ -192,7 +192,12 @@ def send_data(imap, tag) = imap.__send__(:put_string, data)
 
     class RawData < CommandData # :nodoc:
       def initialize(data:)
-        data = split_parts(data)
+        case data
+        in String then data = split_parts(data)
+        in Array  if   data.all? { _1 in RawText | Literal }
+        else
+          raise TypeError, "expected String or Array[#{RawText} | #{Literal}]"
+        end
         super
         validate
       end