amnezia-vpn/amnezia-client

Fork 0

mirror of https://github.com/amnezia-vpn/amnezia-client.git synced 2026-10-14 21:34:26 +00:00

[GH-ISSUE #2937] Split tunneling: a network entered in the site list loses its prefix length and becomes a /32 host #1341

New issue

Open

opened 2026-10-13 20:40:31 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-13 20:40:31 +00:00

Owner

Originally created by @anton-vinogradov on GitHub (Aug 4, 2026).
Original GitHub issue: https://github.com/amnezia-vpn/amnezia-client/issues/2937

Summary

Any IPv4 network typed into the split tunneling site list is stored without its prefix length. 84.252.144.0/20 becomes 84.252.144.0, which the app then applies as a single /32 host. The network is silently reduced to one address, so the exclusion does nothing. There is no error and the UI shows the entry as accepted.

The same happens when a site list is imported from JSON.

Environment


App	AmneziaVPN 5.0.0
OS	macOS 26.5.2 (arm64)
Mode	Sites: All sites except the listed ones

The defect is in shared code, so it is not specific to macOS.

Steps to reproduce

Open split tunneling settings and pick All sites except the listed ones.
Add 84.252.144.0/20.
Connect, then look at the routing table and at the allownets table of the amn pf anchor.

Expected

The list holds 84.252.144.0/20, and the app installs a route and a firewall entry for the whole /20.

Actual

The list holds 84.252.144.0. Both halves are installed for a /32:

84.252.144/32   <gateway>   UGSc   en0

allownets: 84.252.144.0

Everything else in the pipeline is correct — the route and the firewall entry are created, they just cover one address instead of 4096.

Where it happens

IpSplitTunnelingController::normalizeHostname in client/core/controllers/ipSplitTunnelingController.cpp:

normalized = normalized.split("/", Qt::SkipEmptyParts).first();

The line is meant to cut the path off a URL such as example.com/page. It also cuts the prefix length off a CIDR.

Nothing downstream catches it. NetworkUtilities::ipAddressWithSubnetRegExp treats the /NN suffix as optional, so 84.252.144.0 passes validation as a plain address.

IpSplitTunnelingController::importSitesFromJson calls the same function, so importing a list does not work around this.

Suggested fix

Return early when the input already is a valid address or network:

QString IpSplitTunnelingController::normalizeHostname(const QString &hostname) const
{
    QString normalized = hostname.trimmed();
    normalized.replace("https://", "");
    normalized.replace("http://", "");
    normalized.replace("ftp://", "");
    if (NetworkUtilities::checkIpSubnetFormat(normalized)) {
        return normalized;
    }
    normalized = normalized.split("/", Qt::SkipEmptyParts).first();
    return normalized;
}

Existing lists already saved with a stripped prefix will stay broken, so a note in the release notes may be worth it.

Originally created by @anton-vinogradov on GitHub (Aug 4, 2026). Original GitHub issue: https://github.com/amnezia-vpn/amnezia-client/issues/2937 ## Summary Any IPv4 network typed into the split tunneling site list is stored without its prefix length. `84.252.144.0/20` becomes `84.252.144.0`, which the app then applies as a single `/32` host. The network is silently reduced to one address, so the exclusion does nothing. There is no error and the UI shows the entry as accepted. The same happens when a site list is imported from JSON. ## Environment | | | |---|---| | App | AmneziaVPN 5.0.0 | | OS | macOS 26.5.2 (arm64) | | Mode | Sites: *All sites except the listed ones* | The defect is in shared code, so it is not specific to macOS. ## Steps to reproduce 1. Open split tunneling settings and pick *All sites except the listed ones*. 2. Add `84.252.144.0/20`. 3. Connect, then look at the routing table and at the `allownets` table of the `amn` pf anchor. ## Expected The list holds `84.252.144.0/20`, and the app installs a route and a firewall entry for the whole `/20`. ## Actual The list holds `84.252.144.0`. Both halves are installed for a `/32`: ``` 84.252.144/32 <gateway> UGSc en0 ``` ``` allownets: 84.252.144.0 ``` Everything else in the pipeline is correct — the route and the firewall entry are created, they just cover one address instead of 4096. ## Where it happens `IpSplitTunnelingController::normalizeHostname` in `client/core/controllers/ipSplitTunnelingController.cpp`: ```cpp normalized = normalized.split("/", Qt::SkipEmptyParts).first(); ``` The line is meant to cut the path off a URL such as `example.com/page`. It also cuts the prefix length off a CIDR. Nothing downstream catches it. `NetworkUtilities::ipAddressWithSubnetRegExp` treats the `/NN` suffix as optional, so `84.252.144.0` passes validation as a plain address. `IpSplitTunnelingController::importSitesFromJson` calls the same function, so importing a list does not work around this. ## Suggested fix Return early when the input already is a valid address or network: ```cpp QString IpSplitTunnelingController::normalizeHostname(const QString &hostname) const { QString normalized = hostname.trimmed(); normalized.replace("https://", ""); normalized.replace("http://", ""); normalized.replace("ftp://", ""); if (NetworkUtilities::checkIpSubnetFormat(normalized)) { return normalized; } normalized = normalized.split("/", Qt::SkipEmptyParts).first(); return normalized; } ``` Existing lists already saved with a stripped prefix will stay broken, so a note in the release notes may be worth it.

bsakaguchi commented

2026-10-13 20:40:33 +00:00

Author

Owner

@HeisLuka commented on GitHub (Sep 9, 2026):

I checked the current source and Git history, and this specific issue appears to already be fixed.

The reported root cause was correct: IpSplitTunnelingController::normalizeHostname() used to unconditionally split the input on /, so a valid network such as:

84.252.144.0/20

became:

84.252.144.0

before validation/storage.

PR #2973 fixed exactly this path in commit:

b575df05c7074970956e8ff12a50fbe5e43fd55e
fix: fixed adding subnet to ip split tunnel (#2973)

The new logic checks for a valid IP/CIDR before treating / as a URL path separator:

if (NetworkUtilities::ipAddressWithSubnetRegExp().exactMatch(normalized)) {
    return normalized;
}

const QStringList parts =
    normalized.split("/", Qt::SkipEmptyParts);

return parts.isEmpty() ? QString() : parts.first();

The current dev branch still contains this fix.

I also checked the Git ancestry: b575df0 is an ancestor of the 5.0.1.5 tag, so this fix is already included in stable 5.0.1.5 and later, not only in an unreleased development build.

This should also cover the CIDR-prefix-loss part of #2939 and the same /32 symptom reported in #2904 / #2985, since those reports go through the same normalization path.

One caveat: entries that were already saved by 5.0.0.x after the prefix had been stripped cannot be repaired automatically. Once 84.252.144.0/20 has been persisted as just 84.252.144.0, the original /20 is no longer recoverable. Those lists need to be re-added/re-imported from the original CIDR source after upgrading.

The only engineering gap I see is regression coverage: the fixing commit changes the implementation but does not add a test.

Useful cases would be:

84.252.144.0/20 → unchanged
192.168.0.0/16 → unchanged
https://example.com/path → example.com
example.com → unchanged
export/import round-trip preserves CIDR prefix lengths

So unless there is a separate reproduction on 5.0.1.5+, I think #2937 can be closed as fixed by #2973.

@HeisLuka commented on GitHub (Sep 9, 2026): I checked the current source and Git history, and this specific issue appears to already be fixed. The reported root cause was correct: `IpSplitTunnelingController::normalizeHostname()` used to unconditionally split the input on `/`, so a valid network such as: ```text 84.252.144.0/20 ``` became: ```text 84.252.144.0 ``` before validation/storage. PR #2973 fixed exactly this path in commit: `b575df05c7074970956e8ff12a50fbe5e43fd55e` `fix: fixed adding subnet to ip split tunnel (#2973)` The new logic checks for a valid IP/CIDR before treating `/` as a URL path separator: ```cpp if (NetworkUtilities::ipAddressWithSubnetRegExp().exactMatch(normalized)) { return normalized; } const QStringList parts = normalized.split("/", Qt::SkipEmptyParts); return parts.isEmpty() ? QString() : parts.first(); ``` The current `dev` branch still contains this fix. I also checked the Git ancestry: `b575df0` is an ancestor of the `5.0.1.5` tag, so this fix is already included in stable **5.0.1.5 and later**, not only in an unreleased development build. This should also cover the CIDR-prefix-loss part of #2939 and the same `/32` symptom reported in #2904 / #2985, since those reports go through the same normalization path. One caveat: entries that were already saved by 5.0.0.x **after the prefix had been stripped cannot be repaired automatically**. Once `84.252.144.0/20` has been persisted as just `84.252.144.0`, the original `/20` is no longer recoverable. Those lists need to be re-added/re-imported from the original CIDR source after upgrading. The only engineering gap I see is regression coverage: the fixing commit changes the implementation but does not add a test. Useful cases would be: * `84.252.144.0/20` → unchanged * `192.168.0.0/16` → unchanged * `https://example.com/path` → `example.com` * `example.com` → unchanged * export/import round-trip preserves CIDR prefix lengths So unless there is a separate reproduction on 5.0.1.5+, I think #2937 can be closed as fixed by #2973.

No labels

pull-request

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

amnezia-vpn/amnezia-client#1341

No description provided.

Rows
Columns