Skip to content

CodeQL: Net: Comparison result is always the same #5050

@matejk

Description

@matejk

Describe the bug

Reported by CodeQL for Poco 1.15 (pre-release):

https://github.com/pocoproject/poco/security/code-scanning/1625

Net/src/IPAddressImpl.cpp:568

bool IPv6AddressImpl::isLoopback() const
{
	if (isIPv4Mapped())
		return (ByteOrder::fromNetwork(_addr.s6_addr[6]) & 0xFF000000) == 0x7F000000;

Comparison is always false because ... & ... <= 255.

	const UInt16* words = reinterpret_cast<const UInt16*>(&_addr);
	return words[0] == 0 && words[1] == 0 && words[2] == 0 && words[3] == 0 &&

Tool: CodeQL
Rule ID: cpp/constant-comparison

Query
View source

Description

Comparison operations like x >= y or x != y will always return the same result if the ranges of x and y do not overlap. In some cases this can cause an infinite loop. In the example below the loop condition on line 9 is always true because the range of i is [0..5], so the loop will never terminate.

The bounds which were deduced for the left and right operands of the comparison are included in the message as they often make it easier to understand why a result was reported. For example the message for the comparison x >= y might read: "Comparison is always false because x >= 5 and 3 >= y."

Copilot Autofix

The code on line 568 in IPv6AddressImpl::isLoopback() attempts to check whether the IPv4-mapped portion of the IPv6 address is a loopback address (127.0.0.0/8). This should be achieved by reconstructing the mapped IPv4 address from the last four bytes of _addr.s6_addr[12..15], and then checking whether it falls in the range 127.0.0.0/8. The most robust fix is to reconstruct the IPv4 address as a 32-bit integer and compare (ipv4Addr & 0xFF000000) == 0x7F000000. The region of code to change is line 568 in Net/src/IPAddressImpl.cpp, inside the body of IPv6AddressImpl::isLoopback(). No new imports are strictly necessary, as the ByteOrder utility is already present.

Suggested changeset 1
Net/src/IPAddressImpl.cpp

@@ -565,7 +565,14 @@
bool IPv6AddressImpl::isLoopback() const
{
	if (isIPv4Mapped())
		UInt32 ipv4Addr = 
			(static_cast<UInt32>(_addr.s6_addr[12]) << 24) |
			(static_cast<UInt32>(_addr.s6_addr[13]) << 16) |
			(static_cast<UInt32>(_addr.s6_addr[14]) << 8)  |
			static_cast<UInt32>(_addr.s6_addr[15]);
		return (ipv4Addr & 0xFF000000) == 0x7F000000;
	}

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions