Page MenuHomeVyOS Platform

ipaddrcheck / libcidr but on IPv6 network validation
Closed, ResolvedPublicBUG

Description

IPV6 addresses containing a double colon :: two times are treated as valid.

This enables users to configure wrong IPv6 addresses via the VyOS CLI which ises this validator.

ipaddrcheck returns 0 on success

# valid address
root@LR1:~# ipaddrcheck --is-ipv6 2001:db8::1
root@LR1:~# echo $?
0
# invalid address
root@LR1:~# ipaddrcheck --is-ipv6 2001:db8::g
root@LR1:~# echo $?
1
# valid address
root@LR1:~# ipaddrcheck --is-ipv6 2001:db8::1/64
root@LR1:~# echo $?
0
# invalid address (two times '::')
root@LR1:~# ipaddrcheck --is-ipv6 2001::db8::1/64
root@LR1:~# echo $?
0

Details

Difficulty level
Unknown (require assessment)
Version
1.2.0-rolling+201808311253
Why the issue appeared?
Will be filled on close

Event Timeline

c-po assigned this task to dmbaturin.Sep 1 2018, 8:04 AM
c-po triaged this task as High priority.
c-po created this task.
c-po updated the task description. (Show Details)Sep 1 2018, 8:06 AM
c-po added a comment.EditedOct 15 2018, 8:21 AM

https://ci.vyos.net/job/ipaddrcheck/build=jessie64devel.vyos.net/5/console

ipaddrcheck_functions.c
ipaddrcheck_functions.c: In function ‘is_ipv6_cidr’:
ipaddrcheck_functions.c:104:23: error: unknown escape sequence: '\.' [-Werror]
     re = pcre_compile("^(((?:(?:(?:[A-F0-9]{1,4}:){6}|(?=(?:[A-F0-9]{0,4}:){0,6}(?:[0-9]{1,3}\.){3}[0-9]{1,3}$)(([0-9A-F]{1,4}:){0,5}|:)((:[0-9A-F]{1,4}){1,5}:|:)|::(?:[A-F0-9]{1,4}:){5})(?:(?:25[0-5]|2[0-4][0-9]|1[0-9][0-9]|[1-9]?[0-9])\.){3}(?:25[0-5]|2[0-4][0-9]|1[0-9][0-9]|[1-9]?[0-9])|(?:[A-F0-9]{1,4}:){7}[A-F0-9]{1,4}|(?=(?:[A-F0-9]{0,4}:){0,7}[A-F0-9]{0,4}$)(([0-9A-F]{1,4}:){1,7}|:)((:[0-9A-F]{1,4}){1,7}|:)|(?:[A-F0-9]{1,4}:){7}:|:(:[A-F0-9]{1,4}){7}))(\\/\\d{1,3}))$",
                       ^
ipaddrcheck_functions.c:104:23: error: unknown escape sequence: '\.' [-Werror]
ipaddrcheck_functions.c: In function ‘is_ipv6_single’:
ipaddrcheck_functions.c:131:23: error: unknown escape sequence: '\.' [-Werror]
     re = pcre_compile("^(?:(?:(?:[A-F0-9]{1,4}:){6}|(?=(?:[A-F0-9]{0,4}:){0,6}(?:[0-9]{1,3}\.){3}[0-9]{1,3}$)(([0-9A-F]{1,4}:){0,5}|:)((:[0-9A-F]{1,4}){1,5}:|:)|::(?:[A-F0-9]{1,4}:){5})(?:(?:25[0-5]|2[0-4][0-9]|1[0-9][0-9]|[1-9]?[0-9])\.){3}(?:25[0-5]|2[0-4][0-9]|1[0-9][0-9]|[1-9]?[0-9])|(?:[A-F0-9]{1,4}:){7}[A-F0-9]{1,4}|(?=(?:[A-F0-9]{0,4}:){0,7}[A-F0-9]{0,4}$)(([0-9A-F]{1,4}:){1,7}|:)((:[0-9A-F]{1,4}){1,7}|:)|(?:[A-F0-9]{1,4}:){7}:|:(:[A-F0-9]{1,4}){7})$",
                       ^
ipaddrcheck_functions.c:131:23: error: unknown escape sequence: '\.' [-Werror]
cc1: all warnings being treated as errors
Makefile:363: recipe for target 'ipaddrcheck_functions.o' failed
make[3]: *** [ipaddrcheck_functions.o] Error 1
make[3]: Leaving directory '/home/jenkins/workspace/ipaddrcheck/build/jessie64devel.vyos.net/src'
pasik added a subscriber: pasik.Oct 15 2018, 9:52 PM

I've redone the patch, it uses a simpler regex because we do not need mixed-mode. The main issue was it didn't validate lowercase. I had to split the host/mask parsing to do the cidr variant properly. I've added tests and it ran succesfully. PR#2

syncer added a subscriber: syncer.Oct 29 2018, 9:59 AM

@c-po can we mark this as resolved?

dmbaturin closed this task as Resolved.Nov 28 2018, 11:20 PM

Apparently we do not have phabricator integration setup for the ipaddrcheck repo, since it didn't pick this commit up: https://github.com/vyos/ipaddrcheck/commit/21c0775c51da1ca3d4ef6506fca82bce5b334c79

Yes, it's resolved now. I've also added unit tests to prevent this bug from re-appearing.