Code Smell 322 - Don't Keep Request-owned Pointers in Long-lived Variables

In the world of software development, certain practices can lead to subtle bugs and security vulnerabilities. One such practice, highlighted in Code Smell 322, involves the improper handling of pointers that are owned by individual requests within long-lived variables. This issue can cause serious problems, such as reading from freed memory, enabling remote code execution, and introducing hidden side effects.

When a server process handles multiple requests in sequence, a single request may only last for a brief moment. However, if a pointer to a request's memory is stored in a process-wide variable, there is a risk of coupling two different lifetimes together without proper consideration. Once the request ends and its memory is freed, the pointer remains in the variable, potentially leading to unintended consequences when subsequent requests begin.

This type of defect arises due to the interaction between two operations: one that leaves behind stale state and another that inadvertently reads or acts on that stale state. The problem is exacerbated by various factors, including time-outs, uncaught errors, and memory exhaustion, which can all trigger the scenario. Similarly, late writes can repopulate state after shutdown has already cleared it, further complicating the issue.

The defect is particularly challenging to detect because it relies on a sequence of events involving two separate requests handled by the same worker. An attacker would need to orchestrate these two requests in a specific order, with one leaving behind a stale value and the other reading or abusing that value. Under certain memory layouts, this sequence can transform a use-after-free error into a remote code execution vulnerability.

To mitigate this issue, developers must employ several strategies. Firstly, state should be scoped per request, preventing pointers from lingering in long-lived variables. Secondly, state ownership should be centralized to maintain clear boundaries and responsibilities. Thirdly, variables should be initialized before any early consumers can access them during the next request.

Fourthly, references should be cleared before freeing memory, ensuring that no lingering pointers exist. Lastly, every exit path must be carefully analyzed to block any late writes to the state.

Several code samples illustrate the flawed practice and the necessary fixes. The sample code demonstrates how a server process might store a pointer to request memory in a global variable, leading to the stale state issue. The provided fix, however, demonstrates the proper approach, where the first example shows selected excerpts from the actual PHP fix, and the second example provides a design sketch for managing state ownership and cleanup.

The fix includes explicit cleanup operations for each state owner, ensuring that references are cleared, owned resources are released, and the process is prepared for the next request.

To detect such issues, developers can perform a thorough code review, focusing on variables that hold pointers to request memory but exist in process scope. They should compare every startup initialization with its corresponding shutdown reset, looking for any missing variables. Additionally, an instrumented build with AddressSanitizer or Valgrind can help identify stale memory accesses.

By intentionally triggering errors in one request and then injecting allocation failures in the next request, developers can uncover hidden side effects and potential vulnerabilities.