Skip to content

Commit 72d0563

Browse files
committed
chore: fix ipMatch code
1 parent 2e66ca9 commit 72d0563

2 files changed

Lines changed: 106 additions & 7 deletions

File tree

src/util/ip.ts

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828

2929
import { Buffer } from 'buffer/';
3030

31-
const ipv4Regex = /^(\d{1,3}\.){3,3}\d{1,3}$/;
31+
const ipv4Regex = /^(25[0-5]|2[0-4]\d|1\d\d|\d?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|\d?\d)){3}$/;
3232
const ipv6Regex = /^(::)?(((\d{1,3}\.){3}(\d{1,3}){1})?([0-9a-f]){0,4}:{0,2}){1,8}(::)?$/i;
3333

3434
export const ip = {
@@ -131,14 +131,13 @@ export const ip = {
131131
},
132132

133133
isV6Format: function (ip: string): boolean {
134-
return ipv6Regex.test(ip);
134+
return ip.includes(':') && ipv6Regex.test(ip);
135135
},
136136

137137
fromPrefixLen: function (prefixlen: number, family?: string): string {
138+
family = _normalizeFamily(typeof family === 'string' ? family : 'ipv4');
138139
if (prefixlen > 32) {
139140
family = 'ipv6';
140-
} else {
141-
family = _normalizeFamily(typeof family === 'string' ? family : '');
142141
}
143142

144143
let len = 4;
@@ -198,7 +197,7 @@ export const ip = {
198197
},
199198

200199
subnet: function (addr: string, mask: string): any {
201-
const networkAddress = ip.toLong(ip.mask(addr, mask));
200+
const networkBuffer = ip.toBuffer(ip.mask(addr, mask));
202201

203202
// Calculate the mask's length.
204203
const maskBuffer = ip.toBuffer(mask);
@@ -218,7 +217,17 @@ export const ip = {
218217

219218
return {
220219
contains: function (other: string) {
221-
return networkAddress === ip.toLong(ip.mask(other, mask));
220+
let otherBuffer: Buffer;
221+
try {
222+
otherBuffer = ip.toBuffer(ip.mask(other, mask));
223+
} catch {
224+
return false;
225+
}
226+
if (networkBuffer.length !== otherBuffer.length) return false;
227+
for (let i = 0; i < networkBuffer.length; i++) {
228+
if (networkBuffer[i] !== otherBuffer[i]) return false;
229+
}
230+
return true;
222231
},
223232
};
224233
},
@@ -228,7 +237,8 @@ export const ip = {
228237
const addr = cidrParts[0];
229238
if (cidrParts.length !== 2) throw new Error('invalid CIDR subnet: ' + addr);
230239

231-
const mask = ip.fromPrefixLen(parseInt(cidrParts[1], 10));
240+
const family = ip.isV6Format(addr) ? 'ipv6' : 'ipv4';
241+
const mask = ip.fromPrefixLen(parseInt(cidrParts[1], 10), family);
232242

233243
return ip.subnet(addr, mask);
234244
},

test/util.test.ts

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
// limitations under the License.
1414

1515
import { getLogger, logPrint, Util as util } from '../src';
16+
import { ip } from '../src/util/ip';
1617
import { compile } from '@casbin/expression-eval';
1718

1819
test('test enableLog success', () => {
@@ -145,6 +146,94 @@ test('test ipMatchFunc', () => {
145146
expect(util.ipMatchFunc('192.168.2.189', '192.168.1.134/26')).toEqual(false);
146147
});
147148

149+
// CVE fix: IPv4 octet overflow allow-list bypass
150+
// Before fix: 448.168.2.10 was silently reduced to 192.168.2.10 (448 & 0xff = 192),
151+
// allowing bypass of 192.168.2.0/24 allow-lists.
152+
test('test ipMatchFunc - IPv4 octet overflow is rejected', () => {
153+
// Overflowed octets must be treated as invalid IPs, not silently normalized.
154+
expect(() => util.ipMatchFunc('448.168.2.10', '192.168.2.0/24')).toThrow(Error);
155+
expect(() => util.ipMatchFunc('266.0.0.1', '10.0.0.0/16')).toThrow(Error);
156+
expect(() => util.ipMatchFunc('203.0.369.42', '203.0.113.0/24')).toThrow(Error);
157+
// Exact /32 host rule bypass: 459.0.113.42 must NOT match 203.0.113.42/32
158+
expect(() => util.ipMatchFunc('459.0.113.42', '203.0.113.42/32')).toThrow(Error);
159+
// Boundary values: 255 is valid, 256 is not
160+
expect(util.ipMatchFunc('10.0.255.1', '10.0.0.0/16')).toEqual(true);
161+
expect(() => util.ipMatchFunc('10.0.256.1', '10.0.0.0/16')).toThrow(Error);
162+
});
163+
164+
// CVE fix: IPv6 CIDR matching was completely broken — any string (including "abc")
165+
// matched any IPv6 CIDR policy due to toLong() collapsing everything to 0.
166+
test('test ipMatchFunc - IPv6 CIDR containment is correct', () => {
167+
// Positive: addresses that should actually match
168+
expect(util.ipMatchFunc('::1', '::1/128')).toEqual(true);
169+
expect(util.ipMatchFunc('fe80::1', 'fe80::/10')).toEqual(true);
170+
expect(util.ipMatchFunc('fe80::ffff', 'fe80::/10')).toEqual(true);
171+
expect(util.ipMatchFunc('2001:db8::1', '2001:db8::/32')).toEqual(true);
172+
expect(util.ipMatchFunc('fd00::1', 'fd00::/8')).toEqual(true);
173+
174+
// Negative: addresses outside the CIDR must not match
175+
expect(util.ipMatchFunc('fe80::1', '::1/128')).toEqual(false);
176+
expect(util.ipMatchFunc('ffff::1', '::1/128')).toEqual(false);
177+
expect(util.ipMatchFunc('::', '::1/128')).toEqual(false);
178+
expect(util.ipMatchFunc('2001:db8::1', 'fe80::/10')).toEqual(false);
179+
expect(util.ipMatchFunc('::1', 'fe80::/10')).toEqual(false);
180+
expect(util.ipMatchFunc('fe80::1', '2001:db8::/32')).toEqual(false);
181+
expect(util.ipMatchFunc('::1', '2001:db8::/32')).toEqual(false);
182+
expect(util.ipMatchFunc('::1', 'fd00::/8')).toEqual(false);
183+
expect(util.ipMatchFunc('fe80::1', 'fd00::/8')).toEqual(false);
184+
});
185+
186+
// CVE fix: garbage input like "abc" must be rejected, not silently match IPv6 policies.
187+
test('test ipMatchFunc - non-IP strings are rejected as ip1', () => {
188+
expect(() => util.ipMatchFunc('abc', '::1/128')).toThrow(Error);
189+
expect(() => util.ipMatchFunc('abc', 'fe80::/10')).toThrow(Error);
190+
expect(() => util.ipMatchFunc('abc', '2001:db8::/32')).toThrow(Error);
191+
expect(() => util.ipMatchFunc('abc', 'fd00::/8')).toThrow(Error);
192+
expect(() => util.ipMatchFunc('', '::1/128')).toThrow(Error);
193+
});
194+
195+
test('test ip.isV4Format - octet range validation', () => {
196+
// Valid addresses
197+
expect(ip.isV4Format('0.0.0.0')).toEqual(true);
198+
expect(ip.isV4Format('255.255.255.255')).toEqual(true);
199+
expect(ip.isV4Format('192.168.1.1')).toEqual(true);
200+
expect(ip.isV4Format('10.0.0.1')).toEqual(true);
201+
202+
// Overflowed octets must be rejected
203+
expect(ip.isV4Format('256.0.0.1')).toEqual(false);
204+
expect(ip.isV4Format('448.168.2.10')).toEqual(false);
205+
expect(ip.isV4Format('266.0.0.1')).toEqual(false);
206+
expect(ip.isV4Format('0.0.0.256')).toEqual(false);
207+
expect(ip.isV4Format('999.999.999.999')).toEqual(false);
208+
});
209+
210+
test('test ip.isV6Format - rejects non-IPv6 strings', () => {
211+
// Valid IPv6 addresses
212+
expect(ip.isV6Format('::1')).toEqual(true);
213+
expect(ip.isV6Format('fe80::1')).toEqual(true);
214+
expect(ip.isV6Format('2001:db8::')).toEqual(true);
215+
expect(ip.isV6Format('::')).toEqual(true);
216+
217+
// Non-IPv6 strings must be rejected (no colon present)
218+
expect(ip.isV6Format('abc')).toEqual(false);
219+
expect(ip.isV6Format('a')).toEqual(false);
220+
expect(ip.isV6Format('')).toEqual(false);
221+
expect(ip.isV6Format('192.168.1.1')).toEqual(false);
222+
});
223+
224+
test('test ip.cidrSubnet - IPv6 short prefixes use 16-byte mask', () => {
225+
// /8, /10, /32 on IPv6 previously returned a 4-byte IPv4 mask
226+
expect(ip.cidrSubnet('fd00::/8').contains('fd00::1')).toEqual(true);
227+
expect(ip.cidrSubnet('fd00::/8').contains('fe80::1')).toEqual(false);
228+
expect(ip.cidrSubnet('fe80::/10').contains('fe80::1')).toEqual(true);
229+
expect(ip.cidrSubnet('fe80::/10').contains('::1')).toEqual(false);
230+
expect(ip.cidrSubnet('2001:db8::/32').contains('2001:db8::1')).toEqual(true);
231+
expect(ip.cidrSubnet('2001:db8::/32').contains('2001:db9::1')).toEqual(false);
232+
// Invalid input must not match
233+
expect(ip.cidrSubnet('::1/128').contains('abc')).toEqual(false);
234+
expect(ip.cidrSubnet('::1/128').contains('')).toEqual(false);
235+
});
236+
148237
test('test globMatch', () => {
149238
expect(util.globMatch('/foo', '/foo')).toEqual(true);
150239
expect(util.globMatch('/foo', '/foo*')).toEqual(true);

0 commit comments

Comments
 (0)