Skip to content

Avoid using C99 variable length arrays

Tony Finch requested to merge 3201-no-vla into main

From an attacker's point of view, a VLA declaration is essentially a primitive for performing arbitrary arithmetic on the stack pointer. If the attacker can control the size of a VLA they have a very powerful tool for causing memory corruption.

To mitigate this kind of attack, and the more general class of stack clash vulnerabilities, C compilers insert extra code when allocating a VLA to probe the growing stack one page at a time. If these probes hit the stack guard page, the program will crash.

From the point of view of a C programmer, there are a few things to consider about VLAs:

  • If it is important to handle allocation failures in a controlled manner, don't use VLAs. You can use VLAs if it is OK for unreasonable inputs to cause an uncontrolled crash.

  • If the VLA is known to be smaller than some known fixed size, use a fixed size array and a run-time check to ensure it is large enough. This will be more efficient than the compiler's stack probes that need to cope with arbitrary-size VLAs.

  • If the VLA might be large, allocate it on the heap. The heap allocator can allocate multiple pages in one shot, whereas the stack clash probes work one page at a time.

Most of the existing uses of VLAs in BIND are in test code, but there was one instance in named, in the GSS-TSIG verification code. In this case the size of the VLA came from the size of the signature in the request; but it was safe because the code had previously checked that the signature had a reasonable size. However, the safety checks are in the generic TSIG implementation, and the risky VLA usage was in the GSS-specific code, and they are separated by the DST indirection layer, so it wasn't immediately obvious that the risky VLA was in fact safe. And in fact this risky VLA was completely unnecessary, because the GSS signature could be verified in place without being copied to the stack.

This commit removes the GSS-TSIG VLA, and adjusts the style guide and the C compiler flags to allow VLAs in test code but not elsewhere.

Closes #3201 (closed)

Edited by Tony Finch

Merge request reports