Potential Vulnerability in Cloned Code

[pr-hung]

Summary

This PR fixes a potential security vulnerability in cloned code that appears to have missed an upstream security patch.

Details

• Affected file: test/bench/parser/nodejs-parser/http_parser.c
• Upstream fix commit: nodejs/http-parser@7d5c99d
• Clone similarity score: 0.998532

What this PR does

• Applies the upstream security fix logic to the cloned implementation in this repository.

References

• Upstream patch commit: nodejs/http-parser@7d5c99d

Please review and merge this PR to ensure your repository is protected against this potential vulnerability.
Thank you for your time !

[sehe]

This code, as the names suggest, is part of a comparative benchmark only. As such, the code was cloned at the specific time the comparison was done, and represents an "archive copy" for reproducing the results. Of course, we might remove the archive copy and replace it with a revision link to the source repository, or at least clearly mark the files as old and having security issues.

But in short, unless users willingly do not use Boost Beast, but instead use some benchmark code that is not part of Boost Beast, there is no security issue here.

Interestingly, Beast might have corresponding issues with multiple Transfer-Encodings, but that's not the topic of this issue.

[ashtum]

Sorry for the late reply.

Interestingly, Beast might have corresponding issues with multiple Transfer-Encodings, but that's not the topic of this issue.

I think there are two specific issues the Node.js commit addressed:

The presence of both Transfer-Encoding and Content-Length, which is properly handled in Beast here:

beast/include/boost/beast/http/impl/basic_parser.ipp

Lines 742 to 744 in a74967f

	// conflicting field
	if(f_ & flagChunked)
	return bad_content_length();

beast/include/boost/beast/http/impl/basic_parser.ipp

Lines 793 to 798 in a74967f

	if(f_ & flagContentLength)
	{
	// conflicting field
	BOOST_BEAST_ASSIGN_EC(ec, error::bad_transfer_encoding);
	return;
	}

Multiple Transfer-Encoding values with chunked not in last position, which is also handled here:

beast/include/boost/beast/http/impl/basic_parser.ipp

Lines 801 to 810 in a74967f

	auto const v = token_list{value};
	auto const p = std::find_if(v.begin(), v.end(),
	[&](string_view const& s)
	{
	return beast::iequals("chunked"_sv, s);
	});
	if(p == v.end())
	return;
	if(std::next(p) != v.end())
	return;

When chunked is not the last encoding, neither flagChunked nor flagContentLength is set, so Beast silently falls back to read-until-EOF mode, which is correct per RFC 7230 §3.3.3.