Skip to content

Fix incorrect byte shift in IPv4Address byte-array initializer#754

Open
adityasingh2400 wants to merge 1 commit into
apple:mainfrom
adityasingh2400:fix-ipv4-byte-array-init
Open

Fix incorrect byte shift in IPv4Address byte-array initializer#754
adityasingh2400 wants to merge 1 commit into
apple:mainfrom
adityasingh2400:fix-ipv4-byte-array-init

Conversation

@adityasingh2400
Copy link
Copy Markdown

The IPv4Address(_ bytes: [UInt8]) initializer in ContainerizationExtras shifts the third octet by 16 bits instead of 8:

self.value =
    (UInt32(bytes[0]) << 24)
    | (UInt32(bytes[1]) << 16)
    | (UInt32(bytes[2]) << 16)   // should be << 8
    | UInt32(bytes[3])

Because bytes[2] lands in the same bit range as bytes[1], the second octet gets corrupted by the OR, the third octet is dropped, and bits 8 through 15 are always left zero. Concretely, decoding [192, 168, 1, 1] yields 192.169.0.1 instead of 192.168.1.1, and [18, 52, 86, 120] yields 18.118.0.120 instead of 18.52.86.120.

This went unnoticed because the bytes computed property getter uses the correct >> 8 for the third octet, but there was no test exercising the byte-array initializer, so the encode and decode paths were never checked against each other. The sibling IPv6Address(_ bytes:) initializer uses the correct descending shifts (<< 120, << 112, ... << 8, << 0), which is what the IPv4 version should mirror.

The fix changes the third octet shift to 8 bits so the initializer is the exact inverse of the bytes property. I also added two tests to the initializer suite: a valid-input test that asserts both the resulting value and that init(bytes).bytes == bytes round-trips, and an invalid-length test. The round-trip test fails on the current code and passes with the fix.

Verification: swift test --filter ContainerizationExtrasTests passes 221 tests in 26 suites (the IPv4Address suite goes from 23 to 25 tests). The new round-trip test fails before the one-line change and passes after.

The IPv4Address(_ bytes: [UInt8]) initializer shifted the third octet by
16 bits instead of 8. This OR'd the third octet into the same bit range
as the second octet, so the second octet was corrupted and the third was
dropped, while bits 8 through 15 were always left zero.

For example, decoding [192, 168, 1, 1] produced 192.169.0.1 instead of
192.168.1.1. The error was not caught because the byte property getter
(which correctly uses >> 8 for the third octet) had no symmetric test for
the initializer, so the two were never checked against each other. The
IPv6Address byte-array initializer uses the correct descending shifts.

Change the third octet shift to 8 bits so the initializer is the inverse
of the bytes property, and add round-trip and invalid-length tests for
the byte-array initializer.

Signed-off-by: Aditya Singh <adisin650@gmail.com>
@adityasingh2400 adityasingh2400 force-pushed the fix-ipv4-byte-array-init branch from 9ef211d to c7055a9 Compare May 26, 2026 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant