Skip to content

Commit

Permalink
Fix handling of subnet "0.0.0.0/0" in whitelist.
Browse files Browse the repository at this point in the history
"0.0.0.0/0" is a well-known subnet that contains all possible IPv4 addresses.
Envoy whitelist code currently does not handle the "/0" case correctly.

The code uses this formula to compute the subnet mask:
white_list_entry.ipv4_mask_ = ~((1 << (32 - mask)) - 1);

The LHS is an int32_t and the RHS is also computed as a 32-bit value.
However, the (<< 32) operation is invalid for a 32-bit value (undefined behavior).

The fix makes the RHS first compute as a 64-bits value that later gets
truncated to 32-bits on assignment.
  • Loading branch information
Enrico Schiattarella committed Jan 23, 2017
1 parent 8e746bf commit 5e2c2b1
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 1 deletion.
2 changes: 1 addition & 1 deletion source/common/network/utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ IpWhiteList::IpWhiteList(const Json::Object& config) {

Ipv4Entry white_list_entry;
white_list_entry.ipv4_address_ = ntohl(addr.s_addr);
white_list_entry.ipv4_mask_ = ~((1 << (32 - mask)) - 1);
white_list_entry.ipv4_mask_ = ~((1ULL << (32 - mask)) - 1);

// Check to make sure applying the mask to the address equals the address. This can prevent
// user error.
Expand Down
22 changes: 22 additions & 0 deletions test/common/network/utility_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,28 @@ TEST(IpWhiteListTest, Normal) {
EXPECT_FALSE(wl.contains(""));
}

TEST(IpWhiteListTest, MatchAny) {
std::string json = R"EOF(
{
"ip_white_list": [
"0.0.0.0/0"
]
}
)EOF";

Json::ObjectPtr loader = Json::Factory::LoadFromString(json);
IpWhiteList wl(*loader);

EXPECT_TRUE(wl.contains("192.168.3.3"));
EXPECT_TRUE(wl.contains("192.168.3.0"));
EXPECT_TRUE(wl.contains("192.168.3.255"));
EXPECT_TRUE(wl.contains("192.168.0.0"));
EXPECT_TRUE(wl.contains("192.0.0.0"));
EXPECT_TRUE(wl.contains("1.1.1.1"));

EXPECT_FALSE(wl.contains(""));
}

TEST(NetworkUtility, NonNumericResolve) {
EXPECT_THROW(Utility::resolveTCP("localhost", 80), EnvoyException);
}
Expand Down

0 comments on commit 5e2c2b1

Please sign in to comment.