🥅 Test for NULL bytes before sending string args

lib/net/imap/command_data.rb

@@ -15,6 +15,9 @@ def validate_data(data)
       case data
       when nil
       when String
+        if data.include?("\0")
+          raise DataFormatError, "String argument contains NULL byte"
+        end
       when Integer
         # Covers modseq-valzer, which is the largest valid IMAP integer
         if data.negative?
@@ -55,24 +58,32 @@ def send_data(data, tag = nil)
       end
     end
 
+    UNQUOTABLE_CHARS = /\0\r\n/n
+    ASTRING_SPECIALS = /[(){ \x00-\x1f\x7f%*"\\]/n
+    private_constant :UNQUOTABLE_CHARS, :ASTRING_SPECIALS
+
+    # Sends generic strings formatted as +astring+:
+    # * as an atom (note: this is based on +ASTRING-CHAR+, not +ATOM-CHAR+)
+    # * as a quoted string
+    # * as a literal (may send non-synchronizing)
+    #
+    # NOTE: This does not validate that +str+ contains no +NULL+ bytes.
     def send_string_data(str, tag = nil)
       if str.empty?
+        # same as send_quoted_string(str), but incompatible encoding is allowed
         put_string('""')
-      elsif str.match?(/[\r\n]/n)
-        # literal, because multiline
+      elsif str.match?(UNQUOTABLE_CHARS)
         send_literal(str, tag)
       elsif !str.ascii_only?
         if @utf8_strings
-          # quoted string
           send_quoted_string(str)
         else
-          # literal, because of non-ASCII bytes
           send_literal(str, tag)
         end
-      elsif str.match?(/[(){ \x00-\x1f\x7f%*"\\]/n)
-        # quoted string
+      elsif str.match?(ASTRING_SPECIALS)
         send_quoted_string(str)
       else
+        # valid +astring+ atom: non-empty, ASCII only, no ASTRING_SPECIALS
         put_string(str)
       end
     end
@@ -273,6 +284,17 @@ def self.extract_literal(data, binary:, bytesize:, non_sync:)
       private_class_method :extract_literal
     end
 
+    # Please note that +ATOM-CHAR+ and +atom+ are not the same as +ASTRING-char+
+    # and the atom form of +astring+.  +astring+ allows <tt>]</tt>, but +atom+
+    # does not.
+    #
+    # Generic string arguments in Net::IMAP use +astring+, so they will send as
+    # atoms even if they contain <tt>]</tt>.
+    #
+    # This class is used by:
+    # * to validate generic symbol arguments, as the superclass of Flag
+    # * to validate #enable +capabilities+
+    # * to validate #store (and #uid_store) +attr+
     class Atom < CommandData # :nodoc:
       def initialize(**)
         super

test/net/imap/test_imap.rb

@@ -921,6 +921,94 @@ def test_raw_data
     end
   end
 
+  test "sending nil args" do
+    with_fake_server do |server, imap|
+      server.on "TEST", &:done_ok
+      def imap.test_args(*args) = send_command("TEST", *args)
+
+      imap.test_args nil, [nil]
+      assert_equal "NIL (NIL)", server.commands.pop.args
+    end
+  end
+
+  test "sending atom string args (astring-chars)" do
+    with_fake_server do |server, imap|
+      server.on "TEST", &:done_ok
+      def imap.test_args(*args) = send_command("TEST", *args)
+
+      imap.test_args "valid-atoms", %w[foo=bar $baz]
+      assert_equal "valid-atoms (foo=bar $baz)", server.commands.pop.args
+
+      imap.test_args "unquoted-astring", "[resp-specials]"
+      assert_equal "unquoted-astring [resp-specials]", server.commands.pop.args
+    end
+  end
+
+  test "string args don't allow NULL bytes" do
+    with_fake_server do |server, imap|
+      server.on "TEST", &:done_ok
+      def imap.test_args(*args) = send_command("TEST", *args)
+
+      assert_raise_with_message(Net::IMAP::DataFormatError, /NULL byte/) do
+        imap.test_args "NULL=\0"
+      end
+
+      assert_raise_with_message(Net::IMAP::DataFormatError, /NULL byte/) do
+        imap.test_args ["ok", "also ok", "not ok: \0"]
+      end
+    end
+  end
+
+  test "sending quoted string args" do
+    with_fake_server do |server, imap|
+      server.on "TEST", &:done_ok
+      def imap.test_args(*args) = send_command("TEST", *args)
+
+      imap.test_args "empty", "", [""]
+      assert_equal   'empty "" ("")', server.commands.pop.args
+
+      imap.test_args "simple-quotable-specials", "() {} %*"
+      assert_equal('simple-quotable-specials "() {} %*"'.b,
+                   server.commands.pop.args)
+
+      imap.test_args "ascii-ctrl-chars", "\b\x7f"
+      assert_equal("ascii-ctrl-chars \"\b\x7f\"".b, server.commands.pop.args)
+
+      imap.test_args "quoted-specials", ["backslash=\\", 'dquotes=""']
+      assert_equal('quoted-specials ("backslash=\\\\" "dquotes=\\"\\"")'.b,
+                   server.commands.pop.args)
+    end
+  end
+
+  test "sending UTF-8 string args" do
+    with_fake_server(
+      with_extensions: %w[UTF8=ACCEPT LITERAL-],
+      greeting_capabilities: true,
+      capabilities_enablable: %w[UTF8=ACCEPT],
+    ) do |server, imap|
+      server.on "TEST", &:done_ok
+      def imap.test_args(*args) = send_command("TEST", *args)
+
+      # Before enabling UTF-8 strings, with non-synchronizing literals
+      imap.test_args "sync-literal-utf8", ["αβγδε"]
+      assert_equal("sync-literal-utf8 ({10+}\r\nαβγδε)".b,
+                   server.commands.pop.args)
+
+      # Before enabling UTF-8 strings, without non-synchronizing literals
+      imap.config.max_non_synchronizing_literal = -1
+      imap.test_args "sync-literal-utf8", ["αβγδε"]
+      assert_equal("sync-literal-utf8 ({10}\r\nαβγδε)".b,
+                   server.commands.pop.args)
+
+      # After enabling UTF-8 strings
+      imap.enable(:utf8)
+      server.commands.pop.args => ["UTF8=ACCEPT"]
+
+      imap.test_args "quoted-utf8", "αβγδε"
+      assert_equal 'quoted-utf8 "αβγδε"'.b, server.commands.pop.args
+    end
+  end
+
   test("send literal args") do
     with_fake_server(with_extensions: %w[LITERAL-]) do |server, imap|
       # disable automatic non-synchronizing literals