fix: check for invalid characters in path, query and headers

.github/workflows/apisix-test.yml

@@ -21,9 +21,7 @@ jobs:
 
     - name: Install CPAN
       run: |
-        curl -s -L http://xrl.us/cpanm > ../cpanm
-        chmod +x ../cpanm
-        sudo mv ../cpanm /bin/cpanm
+        sudo apt install -y cpanminus
 
     - name: Install Test::Nginx
       run: |

.github/workflows/test.yml

@@ -7,11 +7,11 @@ jobs:
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@v2
-    - uses: leafo/gh-actions-lua@v8
-      with:
-        luaVersion: "luajit-openresty"
-    - uses: leafo/gh-actions-luarocks@v4
-    - run: luarocks install luacheck
+    - name: Install LuaRocks
+      run: |
+        sudo apt -y install lua5.1 liblua5.1-0-dev
+        curl -fsSL https://raw.githubusercontent.com/apache/apisix/master/utils/linux-install-luarocks.sh | bash
+    - run: sudo luarocks install luacheck
     - run: luacheck lib
 
   run_tests:
@@ -22,40 +22,42 @@ jobs:
           - 1.19.3.1
 
     runs-on: ubuntu-latest
-    container:
-      image: openresty/openresty:${{ matrix.openresty_version }}-alpine-fat
-      # --init runs tinit as PID 1 and prevents the 'WARNING: killing the child process' spam from the test suite
-      options: --init
 
     steps:
     - name: Install deps
       run: |
-        apk add --no-cache curl perl bash wget git perl-dev libarchive-tools
-        ln -s /usr/bin/bsdtar /usr/bin/tar
+        wget -O - https://openresty.org/package/pubkey.gpg | sudo apt-key add -
+        echo "deb http://openresty.org/package/ubuntu $(lsb_release -sc) main" | sudo tee /etc/apt/sources.list.d/openresty.list
+        sudo apt-get update
+        sudo apt-get -y install openresty
+        sudo apt-get -y install curl perl bash wget git libperl-dev libarchive-tools
+        curl -fsSL https://raw.githubusercontent.com/apache/apisix/master/utils/linux-install-luarocks.sh | bash
 
     - name: Install CPAN
-      run: curl -s -L http://xrl.us/cpanm > /bin/cpanm && chmod +x /bin/cpanm
+      run: sudo apt install -y cpanminus
 
     - name: Cache
-      uses: actions/cache@v2
+      uses: actions/cache@v4
       with:
         path: |
           ~/.cpan
           ~/.cache
         key: ${{ runner.os }}-${{ matrix.openresty_version }}-cache
 
     - name: Install Test::Nginx
-      run: cpanm -q -n Test::Nginx
+      run: sudo cpanm -q -n Test::Nginx
 
     - name: Install Luacov
-      run: luarocks install luacov
+      run: sudo luarocks install luacov
 
     - uses: actions/checkout@v2
 
     - name: Run tests
       env:
         TEST_COVERAGE: '1'
-      run: /usr/bin/prove -I../test-nginx/lib t/*.t
+      run: |
+        export PATH=/usr/local/openresty/nginx/sbin:/usr/local/openresty/luajit/bin:$PATH
+        /usr/bin/prove -I../test-nginx/lib t/*.t
 
     - name: Coverage
       run: |

lib/resty/http.lua

@@ -10,6 +10,7 @@ local str_lower = string.lower
 local str_upper = string.upper
 local str_find = string.find
 local str_sub = string.sub
+local str_byte = string.byte
 local tbl_concat = table.concat
 local tbl_insert = table.insert
 local ngx_encode_args = ngx.encode_args
@@ -128,6 +129,99 @@ local DEFAULT_PARAMS = {
 
 local DEBUG = false
 
+-- ideated from:
+-- luacheck: push max code line length 300
+-- https://github.com/hyperium/http/blob/60fbf319500def124cabb21c8fe8533cf209ce58/src/uri/path.rs#L484-L540
+-- luacheck: pop
+local is_allowed_path = {}
+for b = 0, 255 do
+    is_allowed_path[b] = (b == 0x21 -- !
+        or (b >= 0x24 and b <= 0x3B) -- $, %, &, ', (, ), *, +, ,, -, ., /, 0-9, :, ;
+        or b == 0x3D -- =
+        or (b >= 0x40 and b <= 0x5F) -- @, A-Z, [, \, ], ^, _
+        or (b >= 0x61 and b <= 0x7A) -- a-z
+        or b == 0x7C -- |
+        or b == 0x7E -- ~
+        or b == 34   -- "
+        or b == 123  -- {
+        or b == 125  -- }
+        or b >= 127) -- UTF-8 / DEL
+end
+
+local function validate_path(value)
+    if type(value) ~= "string" then return false end
+
+    for i = 1, #value do
+        if not is_allowed_path[str_byte(value, i)] then
+            return false
+        end
+    end
+    return true
+end
+
+local is_allowed_query = {}
+for b = 0, 255 do
+    is_allowed_query[b] = is_allowed_path[b]
+end
+-- Query logic matches Path logic, but adds 0x3F (?)
+is_allowed_query[0x3F] = true
+
+local function validate_query(value)
+    if type(value) ~= "string" then return false end
+
+    for i = 1, #value do
+        if not is_allowed_query[str_byte(value, i)] then
+            return false
+        end
+    end
+    return true
+end
+
+-- Pre-compute the Token table (based on Go's httpguts.isTokenTable)
+-- https://github.com/golang/net/blob/60b3f6f8ce12def82ae597aebe9031753198f74d/http/httpguts/httplex.go#L15-L93
+local is_token_char = {}
+local tokens = "!#$%&'*+-.0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ^_`abcdefghijklmnopqrstuvwxyz|~"
+
+for i = 1, #tokens do
+    is_token_char[str_byte(tokens, i)] = true
+end
+
+local function validate_header_name(name)
+    if type(name) ~= "string" or name == "" then return false end
+
+    for i = 1, #name do
+        if not is_token_char[str_byte(name, i)] then
+            return false
+        end
+    end
+    return true
+end
+
+-- Pre-compute the Header Value table
+local is_valid_header_value_char = {}
+for b = 0, 255 do
+    -- Logic: Not a CTL, OR is LWS (Space 32 or Tab 9)
+    -- isCTL: b < 32 or b == 127
+    -- isLWS: b == 32 or b == 9
+    local is_ctl = (b < 32 or b == 127)
+    local is_lws = (b == 32 or b == 9)
+
+    if not is_ctl or is_lws then
+        is_valid_header_value_char[b] = true
+    else
+        is_valid_header_value_char[b] = false
+    end
+end
+
+local function validate_header_value(value)
+    if type(value) ~= "string" then return false end
+    for i = 1, #value do
+        if not is_valid_header_value_char[str_byte(value, i)] then
+            return false
+        end
+    end
+    return true
+end
 
 function _M.new(_)
     local sock, err = ngx_socket_tcp()
@@ -311,19 +405,28 @@ local function _format_request(self, params)
     local version = params.version
     local headers = params.headers or {}
 
+    local path = params.path
+    if not validate_path(path) then
+        return nil, "invalid characters found in path"
+    end
+
     local query = params.query or ""
     if type(query) == "table" then
         query = "?" .. ngx_encode_args(query)
     elseif query ~= "" and str_sub(query, 1, 1) ~= "?" then
         query = "?" .. query
     end
 
+    if not validate_query(query) then
+        return nil, "invalid characters found in query"
+    end
+
     -- Initialize request
     local req = {
         str_upper(params.method),
         " ",
         self.path_prefix or "",
-        params.path,
+        path,
         query,
         HTTP[version],
         -- Pre-allocate slots for minimum headers and carriage return.
@@ -336,14 +439,25 @@ local function _format_request(self, params)
     -- Append headers
     for key, values in pairs(headers) do
         key = tostring(key)
+        if not validate_header_name(key) then
+            return nil, "invalid characters found in header key"
+        end
 
         if type(values) == "table" then
             for _, value in pairs(values) do
+                value = tostring(value)
+                if not validate_header_value(value) then
+                    return nil, "invalid characters found in header value"
+                end
                 req[c] = key .. ": " .. tostring(value) .. "\r\n"
                 c = c + 1
             end
 
         else
+            values = tostring(values)
+            if not validate_header_value(values) then
+                return nil, "invalid characters found in header value"
+            end
             req[c] = key .. ": " .. tostring(values) .. "\r\n"
             c = c + 1
         end
@@ -736,7 +850,11 @@ function _M.send_request(self, params)
     params.headers = headers
 
     -- Format and send request
-    local req = _format_request(self, params)
+    local req, err = _format_request(self, params)
+    if not req then
+        return nil, err
+    end
+
     if DEBUG then ngx_log(ngx_DEBUG, "\n", req) end
     local bytes, err = sock:send(req)