ClickHouse / ClickHouse/clickhouse-cpp
Incorrect byte order conversion in ColumnIPv4::Append() overloads
- Dominant language
- C
- Stars
- 382
- Forks
- 209
- Avg merge
- 3h 58m
- Merged PRs (30d)
- 12
Description
In ColumnIPv4's Append method std::string is converted to uint32_t with inet_pton().
```
void ColumnIPv4::Append(const std::string& str) {
uint32_t address;
if (inet_pton(AF_INET, str.c_str(), &address) != 1)
throw ValidationError("invalid IPv4 format, ip: " + str);
data_->Append(htonl(address));
}
```
After successful conversion htonl() function is used to convert uint32_t from host byte order to network byte order.
From inet_pton man page:
```
This function converts the character string src into a network
address structure in the af address family, then copies the
network address structure to dst. The af argument must be either
AF_INET or AF_INET6. dst is written in **network byte order**.
```
So this function incorrectly stores the address in data_ in host byte order.
Also, it is explicitly specified that overload that receives uint32_t as input expects it's argument to be passed in host byte order.
```
/// @params ip numeric value with host byte order.
void Append(uint32_t ip);
```
And internally it converts it to network byte order with htonl()
```
void ColumnIPv4::Append(uint32_t ip) {
data_->Append(htonl(ip));
}
```
So these two methods overloads write data in different byte orders.
Additionally, it is not documented for overload that receives in_addr which byte order should be used. But looking at the implementation, it's obvious that it expects it to be also in host byte order, which is not intuitive, since all standard functions from libc provide in_addr with network byte order.
```
void ColumnIPv4::Append(in_addr ip) {
data_->Append(htonl(ip.s_addr));
}
```
In our codebase we were incorrectly using `Append(uint32_t ip)`. Despite the comment, we were providing ip in network byte order, but due to symmetrical conversion in
```
in_addr ColumnIPv4::At(size_t n) const {
in_addr addr;
addr.s_addr = ntohl(data_->At(n));
return addr;
}
```
we were getting `in_addr` in network byte order, which was what we were expecting, so everything worked correctly.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.