openresty / openresty/stream-lua-nginx-module

Potential NULL Pointer Dereference in ngx_stream_lua_socket_receiveuntil_iterator

Open
#366 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C
Stars
749
Forks
210
Avg merge
4h 31m
Merged PRs (30d)
3

Description

Hello! I analyzed Nginx modules with Svace static analyzer. It found a potential problem in the code in /stream-lua-nginx-module/src/ngx_stream_lua_socket_tcp.c

Problem Description

The function ngx_stream_lua_socket_receiveuntil_iterator contains a potential null pointer dereference issue. Specifically, the return value of lua_touserdata is used without checking if it is NULL. If lua_touserdata returns NULL (which can happen if the upvalue at lua_upvalueindex(3) is not valid userdata or does not exist), the code will attempt to access cp->state, leading to undefined behavior, such as a segmentation fault or crash.

This issue violates best practices in C programming, where functions that may return NULL should always be checked before dereferencing. Additionally, modern static analysis tools often flag such cases as potential bugs, reducing code reliability and maintainability.

Solution Description

To address this issue, a NULL check was added after calling lua_touserdata. If the returned value (cp) is NULL, the function immediately returns an error with a descriptive message ("invalid compiled pattern object"). This ensures that:

  1. The function handles invalid or missing userdata gracefully.
  2. Potential crashes due to null pointer dereferencing are avoided.
  3. The code adheres to best practices for safe pointer handling in C.
  4. Debugging is easier, as the error message clearly indicates the problem.
Implementation Details
  • After calling lua_touserdata, the code checks if cp is NULL.
  • If cp is NULL, the function returns an error using luaL_error.
Benefits of the Fix
  1. Improved Robustness: The function now safely handles unexpected or invalid input.
  2. Compliance with Best Practices: The fix aligns with standard practices for handling pointers in C.
  3. Better Static Analysis Results: The patch reduces false positives from static analysis tools, improving overall code quality metrics.

By implementing this fix, the function becomes more reliable and less prone to runtime errors, ensuring smoother operation in both development and production environments.

--- ngx_stream_lua_socket_tcp.c	2025-03-07 11:01:36.383897864 +0300
+++ ngx_stream_lua_socket_tcp.c.patched	2025-03-07 11:05:32.971742575 +0300
@@ -4612,6 +4612,10 @@
 
     cp = lua_touserdata(L, lua_upvalueindex(3));
 
+   if (cp == NULL) {
+       return luaL_error(L, "invalid compiled pattern object");
+   }
+
     dd("checking existing state: %d", cp->state);
 
     if (cp->state == -1) {

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in stream-lua-nginx-module/src/ngx_stream_lua_socket_tcp.c at ngx_stream_lua_socket_receiveuntil_iterator, especially the lua_touserdata call and subsequent cp->state access. Verify the invalid-upvalue path is handled safely, then run the repository's existing test suite or stream-module tests if available. Done means the potential NULL dereference is addressed without changing valid iterator behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
c, nginx
Domain
backend, networking
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.