Skip to content

Potential NULL Pointer Dereference in ngx_stream_lua_socket_receiveuntil_iterator #366

@hpkit

Description

@hpkit

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions