Skip to content

mod_vhost_alias: reserve a byte for the terminating NUL in checkspace - #833

Open
arshsmith1 wants to merge 1 commit into
apache:trunkfrom
arshsmith1:vhost-alias-interpolate-nul
Open

arshsmith1 wants to merge 1 commit into
apache:trunkfrom
arshsmith1:vhost-alias-interpolate-nul

Conversation

@arshsmith1

Copy link
Copy Markdown

Following up on the recent "harden vhost_alias" change, there is still a one-byte stack overflow left in the same path.

  1. vhost_alias_interpolate() builds the interpolated VirtualDocumentRoot/VirtualScriptAlias path into a fixed char buf[HUGE_STRING_LEN], and vhost_alias_checkspace() only flushes/resets dest when *pdest + size > buf + HUGE_STRING_LEN. A write that ends exactly at buf + HUGE_STRING_LEN is therefore allowed, leaving dest one byte past the end of buf.
  2. The flush **pdest = '\0' and the terminating *dest = '\0' at the end of the function then write that NUL one byte past buf.

The size of a %N segment comes from the server name (the Host header), so with an ordinary config such as VirtualDocumentRoot /var/www/%0 a request whose Host length makes the accumulated output total exactly HUGE_STRING_LEN reaches the boundary and writes past the buffer. ASAN reports it as a stack-buffer-overflow WRITE of size 1; on a normal build it is a single stray NUL into adjacent stack, so it is only cleanly observable under ASAN and exists on trunk after the recent hardening (hence a normal bug fix per SECURITY.md, not the vulnerability process).

The fix reserves one byte for the terminator by flushing when *pdest + size >= buf + HUGE_STRING_LEN. Output is byte-identical for every in-bounds expansion; only the exact-fill case now flushes one segment earlier instead of running over.

There is no test/modules/mappers home for a pyhttpd test and the single-byte overrun is not observable on a normal (non-ASAN) build, so no functional test is included, matching the shape of the original hardening commit. I verified the overflow and the fix with a standalone ASAN harness over the extracted buffer logic and can share it if useful.


  • This PR does not report or address a security vulnerability
    (see SECURITY.md and
    https://www.apache.org/security/#reporting-a-vulnerability).
  • Code follows the httpd style guide.
  • New log messages (level debug or higher) use an empty APLOGNO() tag;
    numbers are assigned by a committer at merge time (see docs/log-message-tags/README).
  • If the change is user-visible, a changes-entries/*.txt file is included,
    following the template in README.CHANGES (not needed otherwise).
  • Test cases based on pyhttpd are included where appropriate, in the
    test/modules/xxx directory matching the modules/xxx module source,
    or test/modules/core for core server changes. On Unix, verify these
    pass locally with e.g. make check-pytest PYTEST_DIRS=test/modules/xxx.

AI tooling was used to help prepare this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant