Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions Common/headers/tcpverbs.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,11 @@ long tcp_count_connections(void);
boolean tcp_open_stream_name(bigstring hostname, long port, long *stream_id_out);
boolean tcp_name_to_address(bigstring domain_name, long *addr_out);
boolean tcp_address_to_name(long addr, bigstring name_out);
/* #716 item 5: testable helper -- packs a resolved C-string hostname into
the bigstring, falling back to dotted-decimal IP if the hostname exceeds
the 255-byte bigstring limit. Returns true if the hostname fit, false if
the IP fallback was used. */
boolean tcp_address_to_name_pack(long addr, const char *hostname, bigstring name_out);
boolean tcp_address_encode(bigstring ip_string, long *addr_out);
boolean tcp_address_decode(long addr, bigstring ip_string_out);

Expand Down
54 changes: 51 additions & 3 deletions Common/source/tcpverbs.c
Original file line number Diff line number Diff line change
Expand Up @@ -1239,6 +1239,55 @@ boolean tcp_name_to_address(bigstring domain_name, long *addr_out) {
*
* To detect if reverse DNS succeeded, compare the result with the original
* IP address string - if they match, reverse lookup failed. */
/*
* 2026-06-05 JES #716 item 5: helper extracted from tcp_address_to_name so
* the >255-byte hostname truncation path is testable without mocking
* getnameinfo(). Takes a resolved C-string hostname plus the original
* address for the fallback path; writes the bigstring result and returns
* true if the hostname fit, false if it was rejected and the IP fallback
* was used instead.
*
* NI_MAXHOST is 1025 (macOS, Linux) to accommodate non-DNS resolution
* sources (NIS, LDAP, /etc/hosts), but UserTalk bigstring max payload
* is 255. A long PTR record or attacker-controlled reverse lookup result
* would silently truncate via copyctopstring's clamp, producing a partial
* hostname that could confuse UserTalk-level identity checks (e.g.
* "did we connect to evil.com?" if the PTR was crafted as
* "evil.com.padding-padding-...-victim.com" and got clipped past the
* "evil.com" prefix). Surface as an explicit warning and fall back to
* the dotted-decimal IP string -- the same fallback path used when
* getnameinfo() itself fails. Pattern matches
* frontier-cli/window_registry.c:116 and the post-#707 callsites
* updated in PR #714.
*
* Why dotted-decimal IP fallback rather than empty / langerrormessage:
* the function's documented contract is "always returns true; either the
* hostname or the IP string". An empty string risks UserTalk callers
* treating it as a wildcard or skipping an identity check; an error
* breaks the contract and breaks scripts that rely on it. The IP
* literal also cannot be mistaken for a hostname suffix match (a
* substring check for "victim.com" against "192.168.1.1" is
* unambiguous), so fail-closed-to-IP is the safest choice.
*
* Why log_warn omits the hostname: the truncated hostname is
* attacker-controlled and up to 1025 bytes. Logging it raw would create
* a log-injection vector (newlines, ANSI escapes, format specifiers)
* and a size-amplification risk. The addr alone is enough for an
* operator to investigate via separate dig/host queries.
*/
boolean tcp_address_to_name_pack(long addr, const char *hostname,
bigstring name_out) {
if (!copyctopstring(hostname, name_out)) {
log_warn(LOG_COMP_LANG,
"tcp_address_to_name: resolved hostname exceeded 255 bytes; "
"falling back to IP string (addr=%ld)",
addr);
tcp_address_decode(addr, name_out);
return false;
}
return true;
}

/* tcp.addressToName(addr) -> string
*
* NOTE: Boolean return always true by design. This function cannot fail - it either
Expand Down Expand Up @@ -1270,9 +1319,8 @@ boolean tcp_address_to_name(long addr, bigstring name_out) {
return true;
}

copyctopstring(hostname, name_out);

log_info(LOG_COMP_LANG, "tcp_address_to_name: resolved to hostname");
if (tcp_address_to_name_pack(addr, hostname, name_out))
log_info(LOG_COMP_LANG, "tcp_address_to_name: resolved to hostname");

return true;
}
Expand Down
78 changes: 78 additions & 0 deletions tests/tcp_phase1a_unit_tests.c
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,82 @@ TEST(address_encode_invalid_format) {
}
}

/*
* Test 3.8: tcp_address_to_name_pack rejects >255-byte hostname and falls
* back to IP string (issue #716 item 5)
*
* Purpose: When getnameinfo returns a hostname longer than the 255-byte
* bigstring payload limit (NI_MAXHOST is 1025 on macOS/Linux), the legacy
* code path silently truncated via copyctopstring. A long PTR record could
* confuse UserTalk-level identity checks ("did we connect to evil.com?" if
* an attacker crafted "evil.com.<padding>.victim.com" past the 255 cutoff).
* The fix surfaces the truncation as a log warning and falls back to the
* dotted-decimal IP string -- same as the getnameinfo() failure path.
*
* Why a behavioral test is possible: tcp_address_to_name_pack was extracted
* from tcp_address_to_name specifically so this branch can be exercised
* without mocking getnameinfo. The helper takes the resolved C string and
* the original address, so we drive it directly with a synthetic 300-byte
* hostname.
*
* Reference: tcpverbs.c:tcp_address_to_name_pack(), PR #716 item 5
*/
TEST(address_to_name_pack_long_hostname_falls_back_to_ip) {
bigstring name_out;
char long_hostname[400];
long addr = 0xC0A80101L; /* 192.168.1.1 */
boolean fit;

/* Zero the bigstring so a future regression that early-returns without
writing would surface as a deterministic empty string rather than
reading stack garbage. */
memset(name_out, 0, sizeof(name_out));

/* Construct a 300-byte hostname (well over the 255 bigstring limit). */
memset(long_hostname, 'x', 300);
long_hostname[300] = '\0';

fit = tcp_address_to_name_pack(addr, long_hostname, name_out);

/* The helper should report it didn't fit. */
ASSERT_FALSE(fit);

/* And name_out should contain the dotted-decimal IP fallback, NOT a
255-byte truncation of the synthetic hostname. */
char name_cstr[16];
copyptocstring(name_out, name_cstr);
ASSERT_EQ(strcmp(name_cstr, "192.168.1.1"), 0);
}

/*
* Test 3.9: tcp_address_to_name_pack writes the hostname verbatim when it
* fits (issue #716 item 5 -- happy path)
*
* Purpose: Regression guard alongside test 3.8. Verifies the success path
* preserves the hostname exactly when it is under the bigstring limit, so
* the new branch in tcp_address_to_name doesn't accidentally rewrite short
* hostnames as IPs.
*
* Reference: tcpverbs.c:tcp_address_to_name_pack()
*/
TEST(address_to_name_pack_short_hostname_preserved) {
bigstring name_out;
const char *short_hostname = "example.com";
long addr = 0xC0A80101L; /* 192.168.1.1 -- not used on this branch */
boolean fit;

/* Zero the bigstring (see preceding test's rationale). */
memset(name_out, 0, sizeof(name_out));

fit = tcp_address_to_name_pack(addr, short_hostname, name_out);

ASSERT_TRUE(fit);

char name_cstr[64];
copyptocstring(name_out, name_cstr);
ASSERT_EQ(strcmp(name_cstr, "example.com"), 0);
}

/* ========================================================================
* Test Category 4: Error Handling Tests
* ======================================================================== */
Expand Down Expand Up @@ -804,6 +880,8 @@ int main(void) {
TR_RUN(test_address_decode_to_string);
TR_RUN(test_address_roundtrip_conversion);
TR_RUN(test_address_encode_invalid_format);
TR_RUN(test_address_to_name_pack_long_hostname_falls_back_to_ip);
TR_RUN(test_address_to_name_pack_short_hostname_preserved);

printf("\nTest Category 4: Error Handling\n");
TR_RUN(test_error_invalid_stream_id_zero);
Expand Down