From d8d7409749a6934f51b4bb3af0149986cb31aab6 Mon Sep 17 00:00:00 2001
From: Lev Stipakov <lev@openvpn.net>
Date: Thu, 24 Oct 2024 10:28:10 +0300
Subject: [PATCH] Add support for single quote parsing

This enables to use single quotes, for example:

  pull-filter ignore 'route 1.2.3.4'
  verify-x509-name 'vpn.openvpn.net' (quote is not a part of a name)

Single quotes are treated as regular characters when inside
double quotes and vise versa.

OVPN3-921

Signed-off-by: Lev Stipakov <lev@openvpn.net>
---
 openvpn/common/lex.hpp                   | 62 ++++++++++++++++++++----
 openvpn/common/options.hpp               | 24 +++------
 test/unittests/test_optfilt.cpp          | 31 ++++++++++++
 test/unittests/test_parseargv.cpp        | 14 ++++++
 test/unittests/test_verify_x509_name.cpp |  8 +++
 5 files changed, 113 insertions(+), 26 deletions(-)

diff --git a/openvpn/common/lex.hpp b/openvpn/common/lex.hpp
index bc1dc3c1a..d11b1a662 100644
--- a/openvpn/common/lex.hpp
+++ b/openvpn/common/lex.hpp
@@ -38,8 +38,59 @@ struct SpaceMatch
     }
 };
 
+/**
+ * @brief Helper class to handle quote processing
+ *
+ * This class provides functionality to handle single and double quotes within a text.
+ * It allows treating single quotes as regular characters when inside double quotes,
+ * and vice versa.
+ */
+class LexQuoteMixin
+{
+  public:
+    /**
+     * @brief Check if currently inside a quote
+     * @return true if inside single or double quote, false otherwise
+     */
+    bool in_quote() const
+    {
+        return in_squote | in_dquote;
+    }
+
+  protected:
+    /**
+     * @brief Handle a character as a potential quote
+     *
+     * Toggles quote state if `c` is a single or double quote,
+     * considering the current context.
+     *
+     * @param c Character to process
+     * @return true if `c` is treated as a quote, false otherwise
+     */
+    bool handle_quote(char c)
+    {
+        if ((c == '\"') && (!in_squote))
+        {
+            in_dquote = !in_dquote;
+            return true;
+        }
+
+        if ((c == '\'') && (!in_dquote))
+        {
+            in_squote = !in_squote;
+            return true;
+        }
+
+        return false;
+    }
+
+  private:
+    bool in_squote = false; /**< State for single quotes */
+    bool in_dquote = false; /**< State for double quotes */
+};
+
 // A basic lexical analyzer that understands quoting
-class StandardLex
+class StandardLex : public LexQuoteMixin
 {
   public:
     void put(char c)
@@ -56,9 +107,8 @@ class StandardLex
             backslash_ = true;
             ch = -1;
         }
-        else if (c == '\"')
+        else if (handle_quote(c))
         {
-            in_quote_ = !in_quote_;
             ch = -1;
         }
         else
@@ -77,18 +127,12 @@ class StandardLex
     {
         ch = -1;
     }
-
-    bool in_quote() const
-    {
-        return in_quote_;
-    }
     bool in_backslash() const
     {
         return in_backslash_;
     }
 
   private:
-    bool in_quote_ = false;
     bool backslash_ = false;
     bool in_backslash_ = false;
     int ch = -1;
diff --git a/openvpn/common/options.hpp b/openvpn/common/options.hpp
index d74fd365f..030e00d39 100644
--- a/openvpn/common/options.hpp
+++ b/openvpn/common/options.hpp
@@ -540,13 +540,10 @@ class OptionList : public std::vector<Option>, public RCCopyable<thread_unsafe_r
     typedef StandardLex Lex;
 
     // special lex filter that recognizes end-of-line comments
-    class LexComment
+    class LexComment : public LexQuoteMixin
     {
       public:
-        LexComment()
-            : in_quote_(false), in_comment(false), backslash(false), ch(-1)
-        {
-        }
+        LexComment() = default;
 
         void put(char c)
         {
@@ -564,12 +561,11 @@ class OptionList : public std::vector<Option>, public RCCopyable<thread_unsafe_r
                 backslash = true;
                 ch = -1;
             }
-            else if (c == '\"')
+            else if (handle_quote(c))
             {
-                in_quote_ = !in_quote_;
                 ch = -1;
             }
-            else if (is_comment(c) && !in_quote_)
+            else if (is_comment(c) && !in_quote())
             {
                 in_comment = true;
                 ch = -1;
@@ -593,16 +589,10 @@ class OptionList : public std::vector<Option>, public RCCopyable<thread_unsafe_r
             ch = -1;
         }
 
-        bool in_quote() const
-        {
-            return in_quote_;
-        }
-
       private:
-        bool in_quote_;
-        bool in_comment;
-        bool backslash;
-        int ch;
+        bool in_comment = false;
+        bool backslash = false;
+        int ch = -1;
     };
 
     class Limits
diff --git a/test/unittests/test_optfilt.cpp b/test/unittests/test_optfilt.cpp
index c16ea79cd..07bbba7f6 100644
--- a/test/unittests/test_optfilt.cpp
+++ b/test/unittests/test_optfilt.cpp
@@ -152,6 +152,37 @@ TEST(PushedOptionsFilter, PullFilterMalformedLong)
     ASSERT_THROW(PushedOptionsFilter x(cfg), option_error);
 }
 
+TEST(PushedOptionsFilter, PullFilterSingleQuote)
+{
+    OptionList cfg;
+    cfg.parse_from_config("pull-filter ignore 'route 1.2.3.4'", nullptr);
+    cfg.update_map();
+    PushedOptionsFilter filter(cfg);
+
+    OptionList src;
+    OptionList dst;
+
+    dst.extend(src, &filter);
+
+    testLog->startCollecting();
+    src.parse_from_config("route 1.1.1.1\nroute 2.2.2.2\nroute 1.2.3.4", nullptr);
+    dst.extend(src, &filter);
+    std::string filter_output(testLog->stopCollecting());
+
+    ASSERT_EQ(2u, dst.size())
+        << "Too many options have been accepted by --pull-filter" << std::endl
+        << filter_output;
+}
+
+TEST(PushedOptionsFilter, PullFilterMisplacedQuote)
+{
+    OptionList cfg;
+    cfg.parse_from_config("pull-filter ignore 'a b' c", nullptr);
+    cfg.update_map();
+
+    ASSERT_THROW(PushedOptionsFilter x(cfg), option_error);
+}
+
 TEST(PushedOptionsFilter, PullFilterIgnoreAll)
 {
     OptionList cfg;
diff --git a/test/unittests/test_parseargv.cpp b/test/unittests/test_parseargv.cpp
index f1e8886ca..55a94c892 100644
--- a/test/unittests/test_parseargv.cpp
+++ b/test/unittests/test_parseargv.cpp
@@ -234,3 +234,17 @@ TEST(argv, parsetest1)
     options_csv_test("V4,dev-type tun,link-mtu 1558,tun-mtu 1500,proto UDPv4,comp-lzo,keydir 1,cipher AES-256-CBC,auth SHA1,keysize 256,tls-auth,key-method 2,tls-client",
                      "");
 }
+
+TEST(argv, QuotesTest)
+{
+    auto str = "\"abc,def\",\'abc,def\',\"abc',def\",\'abc\"def\',\'abc\"";
+    std::vector<std::string> list = Split::by_char<std::vector<std::string>, StandardLex, Split::NullLimit>(str, ',');
+    std::stringstream s;
+    std::copy(list.begin(), list.end(), std::ostream_iterator<std::string>(s, "\n"));
+    ASSERT_EQ("\"abc,def\"\n"
+              "\'abc,def'\n"
+              "\"abc',def\"\n"
+              "\'abc\"def\'\n"
+              "\'abc\"\n",
+              s.str());
+}
diff --git a/test/unittests/test_verify_x509_name.cpp b/test/unittests/test_verify_x509_name.cpp
index 14b2909ca..311bdaf96 100644
--- a/test/unittests/test_verify_x509_name.cpp
+++ b/test/unittests/test_verify_x509_name.cpp
@@ -84,6 +84,14 @@ TEST(VerifyX509Name, config_correct_name)
     VerifyX509Name ok_name(parse_testcfg(config));
 }
 
+TEST(VerifyX509Name, config_squote)
+{
+    // ensure that single quote is not treated as name part
+    std::string config = "verify-x509-name 'server.example.org'";
+    VerifyX509Name verify(parse_testcfg(config));
+    ASSERT_TRUE(verify.verify("server.example.org"));
+}
+
 TEST(VerifyX509Name, config_correct_name_prefix)
 {
     // Correct - type: name-prefix