Security review fixes (v3)

[alandefreitas]

• After Security review fixes #982, Boost.URL underwent a third security assessment (Run 39) by Laurel Lye Systems Engineering on April 2, 2026.
• This PR tracks the triage and resolution of all 15 findings from the report.
• Each finding was manually reviewed against the source code and categorized as a confirmed bug (and fixed), a false positive, or a deliberate design choice.
• AI was used only as a supporting tool to help summarize and organize the triage results, propose new test cases, and provide an extra review on each of the code fixes.
• Of the 15 findings, 4 pointed to confirmed bugs resulting in 4 fix commits; the remaining 11 were false positives.
• Most false positives stem from the audit tool not recognizing pre-validation of format strings (parse_pattern / replacement_field_rule validates {/} matching and format specs before detail functions run), atomic types behind #ifdef guards, or preconditions guaranteed by callers.

Summary

v3 (Apr 2, 2026)

Verdict	HIGH	MEDIUM	LOW	Total
FIXED	0	1	3	4
FALSE POSITIVE	4	6	1	11
Total	4	7	4	15

All Fixes (4 commits)

#	Commit	Severity	Description
1	7cd6702	MEDIUM	segments_iter_impl decoded-length in decrement()
2	b55336c	LOW	param noexcept on throwing constructor
3	7c0665d	LOW	string_view_base noexcept on throwing operator std::string()
4	003696d	LOW	url_view memcpy with null source when size is zero

Finding Details

Per-finding triage with a verdict and a rationale for each individual finding.

HIGH (4 findings)

H1. Data race on reference count decrement in release()

• Source: v3
• Location: include/boost/url/grammar/impl/recycled.hpp:78
• Issue: Audit claims refs is a plain std::size_t with no synchronization.
• FALSE POSITIVE: refs is std::atomic<std::size_t> when BOOST_URL_DISABLE_THREADS is not defined (the default). The plain std::size_t branch only applies when threads are explicitly disabled. Same finding was triaged as false positive in v2 (PR Security review fixes #982).

H2. Non-atomic reference count increment in copy constructor

• Source: v3
• Location: include/boost/url/grammar/impl/recycled.hpp:149
• Issue: Same claim as H1 applied to the copy constructor's ++p_->refs.
• FALSE POSITIVE: Same root cause as H1. The increment is atomic when threads are enabled.

H3. Integer overflow in encoded_size leads to heap buffer overflow

• Source: v3
• Location: include/boost/url/impl/encode.hpp:60
• Issue: encoded_size() can overflow std::size_t for very large inputs.
• FALSE POSITIVE: If encoded_size() overflows std::size_t, the encoded result would exceed the maximum representable size and could not be stored in any string or buffer. The overflow means the input already violates the precondition that the encoded output fits in std::size_t.

H4. Out-of-bounds read when format string ends with '{'

• Source: v3
• Location: src/detail/pct_format.cpp:54
• Issue: After ++it1, code dereferences *it1 with only BOOST_ASSERT(it1 != end) as guard.
• FALSE POSITIVE: Format strings are pre-validated by parse_pattern, which uses replacement_field_rule to require every { to have a matching }. A format string ending with an unmatched { fails validation before pct_vmeasure/pct_vformat are ever called. The BOOST_ASSERT correctly documents this precondition.

MEDIUM (7 findings)

M1. Out-of-bounds read after align/fill in format_args

• Source: v3
• Location: src/detail/format_args.cpp:127
• Issue: After consuming fill/align characters, subsequent code dereferences *it without checking it != end.
• FALSE POSITIVE: The format spec is pre-validated by parse_pattern / replacement_field_rule, which requires a closing }. None of the format spec parsing steps (fill/align, width, type) can consume }, so it always reaches } before end. Verified by reverting the fix and confirming the test cases still throw (from parse_pattern, not from format_args).

M2. Out-of-bounds read in integer_formatter_impl::parse

• Source: v3
• Location: src/detail/format_args.cpp:265
• Issue: Same pattern as M1 in the integer formatter.
• FALSE POSITIVE: Same reasoning as M1. The format spec parsing always reaches } before end because parse_pattern validates the format string structure. Verified by reverting the fix: tests still pass.

M3. Unchecked .value() call on port parse result

• Source: v3
• Location: src/detail/pattern.cpp:232
• Issue: Calls .value() on parse result without checking for error.
• FALSE POSITIVE: port_part_rule_t::parse() never returns an error. It returns success with has_port = false when there is no port, and success with the parsed port otherwise. The string being parsed (":digits") was just written by the formatting code above, so it is always valid.

M4. Out-of-bounds read in ci_string with different-size arguments

• Source: v3 (reclassified from HIGH)
• Location: src/grammar/ci_string.cpp:30
• Issue: detail::ci_is_equal loops using s0.size() as bound, which would read past s1 if sizes differ.
• FALSE POSITIVE: The public ci_is_equal(string_view, string_view) in ci_string.hpp checks s0.size() != s1.size() and returns false before calling the detail function. The detail function is never reached with mismatched sizes.

M5. Incorrect decoded-length calculation in segments_iter_impl decrement()

• Source: v3
• Location: include/boost/url/detail/impl/segments_iter_impl.hpp:209
• Issue: When iterating over path segments like /a%20b/c, the iterator tracks how long each segment is when decoded (dn). %20 is a space, so a%20b decoded is a b (3 characters). When going backwards to the first segment, the code did not recalculate dn. It still held the value from whatever segment we were on before (c, which has dn=1). The segment itself displayed correctly (because s_ recalculates independently via the pct_string_view converting constructor), but dn was wrong. So the next time going forward, the iterator added the wrong number to decoded_prefix (the running total of decoded characters before the current segment).
• FIXED (7cd6702, "fix: segments_iter_impl decoded-length in decrement()"): Replaced both the manual string_view construction (index==0) and the backwards inline scan (index>0) with calls to update(), which properly sets dn, next, and s_. Also added missing testObservers() call in test suite.

M6. Potential size_t overflow in range_rule element counter

• Source: v3 (reclassified from LOW)
• Location: include/boost/url/grammar/impl/range_rule.hpp:780
• Issue: Counter n could overflow if parsing an extremely large number of elements.
• FALSE POSITIVE: Each parsed element consumes at least one byte of input. A string_view cannot hold SIZE_MAX bytes on any real system, so n can never reach the overflow threshold. The overflow is physically impossible.

M7. Out-of-bounds read in pct_format after format handler returns

• Source: v3 (reclassified from HIGH)
• Location: src/detail/pct_format.cpp:107
• Issue: After a format handler returns, pctx.begin() is dereferenced with BOOST_ASSERT(*it1 == '}') as the only guard.
• FALSE POSITIVE: Same reasoning as H4. The format string is pre-validated by parse_pattern / replacement_field_rule, which requires every { to have a matching }. By the time pct_vmeasure/pct_vformat run, the closing } is guaranteed to exist. The BOOST_ASSERT correctly documents this precondition.

LOW (4 findings)

L1. noexcept constructor performs potentially-throwing std::string allocations

• Source: v3 (reclassified from MEDIUM)
• Location: include/boost/url/param.hpp:359
• Issue: The param struct has std::string members (key and value). Its constructor takes string_view arguments and copies them into those members. Copying into a std::string allocates memory, and allocation can fail with bad_alloc. But the constructor was marked noexcept, which means if the allocation fails, instead of throwing an exception that the caller could catch, the program calls std::terminate() and crashes with no recovery possible.
• FIXED (b55336c, "fix: param noexcept on throwing constructor"): Removed incorrect noexcept. Verified with static_assert(!std::is_nothrow_constructible<...>) which fails to compile if the fix is reverted.

L2. No validation that minimum count N <= maximum count M in range_rule

• Source: v3
• Location: include/boost/url/grammar/range_rule.hpp:524
• Issue: Constructor doesn't validate that min <= max count.
• FALSE POSITIVE: Passing N > M is a caller error, but the parse logic handles it gracefully (always returns error::mismatch). No UB, no crash, no incorrect results. The missing BOOST_ASSERT is a quality-of-life improvement for debugging, not a functional defect.

L3. noexcept on operator std::string() that can throw

• Source: v3
• Location: include/boost/url/grammar/string_view_base.hpp:162
• Issue: string_view_base has operator std::string() which creates a std::string from the view's contents. Same problem as L1: constructing a std::string allocates memory, can throw bad_alloc, but the function was marked noexcept. Same consequence: unrecoverable crash instead of catchable exception.
• FIXED (7c0665d, "fix: string_view_base noexcept on throwing operator std::string()"): Removed incorrect noexcept.

L4. std::memcpy with potentially null source pointer when size is zero

• Source: v3
• Location: include/boost/url/impl/url_view.hpp:70
• Issue: url_view::persist() copies the URL's character buffer using std::memcpy(dest, data(), size()). If the url_view is default-constructed (empty), data() is nullptr and size() is 0. Copying zero bytes from null seems harmless, but the C and C++ standards say passing nullptr to memcpy is undefined behavior even when the size is zero. Sanitizers (UBSan) flag this.
• FIXED (003696d, "fix: url_view memcpy with null source when size is zero"): Added if(size()) guard.

[cppalliance-bot]

An automated preview of the documentation is available at https://988.url.prtest2.cppalliance.org/index.html

If more commits are pushed to the pull request, the docs will rebuild at the same URL.

2026-04-15 15:11:19 UTC

[cppalliance-bot]

GCOVR code coverage report https://988.url.prtest2.cppalliance.org/gcovr/index.html
LCOV code coverage report https://988.url.prtest2.cppalliance.org/genhtml/index.html
Coverage Diff Report https://988.url.prtest2.cppalliance.org/diff-report/index.html

Build time: 2026-04-15 15:18:35 UTC