Fix: Lua scripts cannot read whole collection at once

CHANGES

@@ -1,6 +1,8 @@
 v3.x.y - YYYY-MMM-DD (to be released)
 -------------------------------------
 
+  - Fix: Lua scripts cannot read whole collection at once
+    [Issue #2900 - @udi-aharon, @airween, @martinhsv]
   - Fix: quoted Include config with wildcard
     [Issue #2905 - @wiseelf, @airween, @martinhsv]
   - Support isolated PCRE match limits

src/variables/variable.h

@@ -179,11 +179,95 @@ class VariableMonkeyResolution {
     static void stringMatchResolveMulti(Transaction *t,
         const std::string &variable,
         std::vector<const VariableValue *> *l) {
-        size_t collection = variable.find(".");
+        size_t collection_delimiter_offset = variable.find(".");
-        if (collection == std::string::npos) {
+        if (collection_delimiter_offset == std::string::npos) {
-            collection = variable.find(":");
+            collection_delimiter_offset = variable.find(":");
         }
-        if (collection == std::string::npos) {
+        std::string col; // collection name excluding individual variable specification
+        std::string var; // variable within the collection
+        if (collection_delimiter_offset == std::string::npos) {
+            col = variable;
+        } else {
+            col = std::string(variable, 0, collection_delimiter_offset);
+            var = std::string(variable, collection_delimiter_offset + 1,
+                variable.length() - (collection_delimiter_offset + 1));
+	}
+
+        // First check if the request is for a collection of type AnchoredSetVariable
+        AnchoredSetVariable* anchoredSetVariable = NULL;
+        if (comp(col, "ARGS")) {
+            anchoredSetVariable = &t->m_variableArgs;
+        } else if (comp(col, "ARGS_GET")) {
+            anchoredSetVariable = &t->m_variableArgsGet;
+        } else if (comp(col, "ARGS_POST")) {
+            anchoredSetVariable = &t->m_variableArgsPost;
+        } else if (comp(col, "FILES_SIZES")) {
+            anchoredSetVariable = &t->m_variableFilesSizes;
+        } else if (comp(col, "FILES_NAMES")) {
+            anchoredSetVariable = &t->m_variableFilesNames;
+        } else if (comp(col, "FILES_TMP_CONTENT")) {
+            anchoredSetVariable = &t->m_variableFilesTmpContent;
+        } else if (comp(col, "MULTIPART_FILENAME")) {
+            anchoredSetVariable = &t->m_variableMultipartFileName;
+        } else if (comp(col, "MULTIPART_NAME")) {
+            anchoredSetVariable = &t->m_variableMultipartName;
+        } else if (comp(col, "MATCHED_VARS_NAMES")) {
+            anchoredSetVariable = &t->m_variableMatchedVarsNames;
+        } else if (comp(col, "MATCHED_VARS")) {
+            anchoredSetVariable = &t->m_variableMatchedVars;
+        } else if (comp(col, "FILES")) {
+            anchoredSetVariable = &t->m_variableFiles;
+        } else if (comp(col, "REQUEST_COOKIES")) {
+            anchoredSetVariable = &t->m_variableRequestCookies;
+        } else if (comp(col, "REQUEST_HEADERS")) {
+            anchoredSetVariable = &t->m_variableRequestHeaders;
+        } else if (comp(variable, "REQUEST_HEADERS_NAMES")) {
+            anchoredSetVariable = &t->m_variableRequestHeadersNames;
+        } else if (comp(col, "RESPONSE_HEADERS")) {
+            anchoredSetVariable = &t->m_variableResponseHeaders;
+        } else if (comp(variable, "RESPONSE_HEADERS_NAMES")) {
+            anchoredSetVariable = &t->m_variableResponseHeadersNames;
+        } else if (comp(col, "GEO")) {
+            anchoredSetVariable = &t->m_variableGeo;
+        } else if (comp(col, "REQUEST_COOKIES_NAMES")) {
+            anchoredSetVariable = &t->m_variableRequestCookiesNames;
+        } else if (comp(col, "FILES_TMPNAMES")) {
+            anchoredSetVariable = &t->m_variableFilesTmpNames;
+        }
+        if (anchoredSetVariable != NULL) {
+            if (collection_delimiter_offset == std::string::npos) {
+                anchoredSetVariable->resolve(l);
+            } else {
+                anchoredSetVariable->resolve(var, l);
+            }
+            return;
+        }
+
+        // Next check for collection of type AnchoredSetVariableTranslationProxy
+        AnchoredSetVariableTranslationProxy* anchoredSetVariableTranslationProxy = NULL;
+        if (comp(col, "ARGS_NAMES")) {
+            anchoredSetVariableTranslationProxy = &t->m_variableArgsNames;
+        } else if (comp(col, "ARGS_GET_NAMES")) {
+            anchoredSetVariableTranslationProxy = &t->m_variableArgsGetNames;
+        } else if (comp(col, "ARGS_POST_NAMES")) {
+            anchoredSetVariableTranslationProxy = &t->m_variableArgsPostNames;
+        }
+        if (anchoredSetVariableTranslationProxy != NULL) {
+            if (collection_delimiter_offset == std::string::npos) {
+                anchoredSetVariableTranslationProxy->resolve(l);
+            } else {
+                anchoredSetVariableTranslationProxy->resolve(var, l);
+            }
+            return;
+        }
+
+
+        // It could still be a non-collection variable, but in that case
+        // there should not be a request for a variable-within-a-collection
+        if (collection_delimiter_offset != std::string::npos) {
+            throw std::invalid_argument("Variable not found.");
+        }
+
         if (comp(variable, "RESPONSE_CONTENT_TYPE")) {
             t->m_variableResponseContentType.evaluate(l);
         } else if (comp(variable, "ARGS_COMBINED_SIZE")) {
@@ -289,58 +373,6 @@ class VariableMonkeyResolution {
         } else {
             throw std::invalid_argument("Variable not found.");
         }
-        } else {
-            std::string col = std::string(variable, 0, collection);
-            std::string var = std::string(variable, collection + 1,
-                variable.length() - (collection + 1));
-            if (comp(col, "ARGS")) {
-                t->m_variableArgs.resolve(var, l);
-            } else if (comp(variable, "ARGS_NAMES")) {
-                t->m_variableArgsNames.resolve(var, l);
-            } else if (comp(variable, "ARGS_GET_NAMES")) {
-                t->m_variableArgsGetNames.resolve(var, l);
-            } else if (comp(variable, "ARGS_POST_NAMES")) {
-                t->m_variableArgsPostNames.resolve(var, l);
-            } else if (comp(col, "ARGS_GET")) {
-                t->m_variableArgsGet.resolve(var, l);
-            } else if (comp(col, "ARGS_POST")) {
-                t->m_variableArgsPost.resolve(var, l);
-            } else if (comp(col, "FILES_SIZES")) {
-                t->m_variableFilesSizes.resolve(var, l);
-            } else if (comp(col, "FILES_NAMES")) {
-                t->m_variableFilesNames.resolve(var, l);
-            } else if (comp(col, "FILES_TMP_CONTENT")) {
-                t->m_variableFilesTmpContent.resolve(var, l);
-            } else if (comp(col, "MULTIPART_FILENAME")) {
-                t->m_variableMultipartFileName.resolve(var, l);
-            } else if (comp(col, "MULTIPART_NAME")) {
-                t->m_variableMultipartName.resolve(var, l);
-            } else if (comp(col, "MATCHED_VARS_NAMES")) {
-                t->m_variableMatchedVarsNames.resolve(var, l);
-            } else if (comp(col, "MATCHED_VARS")) {
-                t->m_variableMatchedVars.resolve(var, l);
-            } else if (comp(col, "FILES")) {
-                t->m_variableFiles.resolve(var, l);
-            } else if (comp(col, "REQUEST_COOKIES")) {
-                t->m_variableRequestCookies.resolve(var, l);
-            } else if (comp(col, "REQUEST_HEADERS")) {
-                t->m_variableRequestHeaders.resolve(var, l);
-            } else if (comp(variable, "REQUEST_HEADERS_NAMES")) {
-                t->m_variableRequestHeadersNames.resolve(var, l);
-            } else if (comp(col, "RESPONSE_HEADERS")) {
-                t->m_variableResponseHeaders.resolve(var, l);
-            } else if (comp(variable, "RESPONSE_HEADERS_NAMES")) {
-                t->m_variableResponseHeadersNames.resolve(var, l);
-            } else if (comp(col, "GEO")) {
-                t->m_variableGeo.resolve(var, l);
-            } else if (comp(col, "REQUEST_COOKIES_NAMES")) {
-                t->m_variableRequestCookiesNames.resolve(var, l);
-            } else if (comp(col, "FILES_TMPNAMES")) {
-                t->m_variableFilesTmpNames.resolve(var, l);
-            } else {
-                throw std::invalid_argument("Variable not found.");
-            }
-        }
     }
 
     static std::string stringMatchResolve(Transaction *t,

test/test-cases/data/match-getvars-args.lua

@@ -0,0 +1,13 @@
+function main()
+    local d = m.getvars("ARGS");
+    local size = #d;
+    m.log(9,"ARGS count read =" .. tostring(size));
+
+    ret = nil
+
+    if ( #d == 2 ) then
+        return nil
+    end
+
+    return "Unexpected result"
+end

test/test-cases/regression/collection-lua.json

@@ -2,7 +2,7 @@
   {
     "enabled":1,
     "version_min":300000,
-    "title":"Testing LUA :: m.set TX (1/6)",
+    "title":"Testing LUA :: m.set TX (1/7)",
     "resource":"lua",
     "client":{
       "ip":"200.249.12.31",
@@ -44,7 +44,7 @@
   {
     "enabled":1,
     "version_min":300000,
-    "title":"Testing LUA :: m.set IP (2/6)",
+    "title":"Testing LUA :: m.set IP (2/7)",
     "resource":"lua",
     "client":{
       "ip":"200.249.12.31",
@@ -86,7 +86,7 @@
   {
     "enabled":1,
     "version_min":300000,
-    "title":"Testing LUA :: m.set GLOBAL (3/6)",
+    "title":"Testing LUA :: m.set GLOBAL (3/7)",
     "resource":"lua",
     "client":{
       "ip":"200.249.12.31",
@@ -128,7 +128,7 @@
   {
     "enabled":1,
     "version_min":300000,
-    "title":"Testing LUA :: m.set RESOURCE (4/6)",
+    "title":"Testing LUA :: m.set RESOURCE (4/7)",
     "resource":"lua",
     "client":{
       "ip":"200.249.12.31",
@@ -170,7 +170,7 @@
    {
     "enabled":1,
     "version_min":300000,
-    "title":"Testing LUA :: m.set SESSION (5/6)",
+    "title":"Testing LUA :: m.set SESSION (5/7)",
     "resource":"lua",
     "client":{
       "ip":"200.249.12.31",
@@ -212,7 +212,7 @@
    {
     "enabled":1,
     "version_min":300000,
-    "title":"Testing LUA :: m.set USER (6/6)",
+    "title":"Testing LUA :: m.set USER (6/7)",
     "resource":"lua",
     "client":{
       "ip":"200.249.12.31",
@@ -250,5 +250,43 @@
       "SecRuleScript test-cases/data/setvar.lua \"id:2,pass\"",
       "SecRule USER.lua_set_var \"@contains 2\" \"id:3,t:none\""
     ]
+ },
+   {
+    "enabled":1,
+    "version_min":300000,
+    "title":"Testing LUA :: m.getvars ARGS (8/8)",
+    "resource":"lua",
+    "client":{
+      "ip":"200.249.12.31",
+      "port":123
+    },
+    "server":{
+      "ip":"200.249.12.31",
+      "port":80
+    },
+    "request":{
+      "headers":{
+        "Host":"localhost",
+        "User-Agent":"My sweet little browser",
+        "Accept":"*/*",
+        "Content-Length": "0"
+      },
+      "uri":"/whee?parm1=a&parm2=b",
+      "method":"GET",
+      "body": [ ]
+    },
+    "response":{
+      "headers":{},
+      "body":[
+        "no need."
+      ]
+    },
+    "expected":{
+      "http_code": 200
+    },
+    "rules":[
+      "SecRuleEngine On",
+      "SecRuleScript test-cases/data/match-getvars-args.lua \"id:2,phase:2,deny,status:403\""
+    ]
  }
 ]