Skip to content

Commit

Permalink
dhcpd: fix ip option parsing
Browse files Browse the repository at this point in the history
  • Loading branch information
ainar-g committed Feb 16, 2021
1 parent f68f6c9 commit e17e003
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 12 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@ and this project adheres to
## [v0.105.2] - 2021-02-24
-->

### Fixed

- Wrong parsing of DHCP options of the `ip` type ([#2688]).

[#2688]: https://github.com/AdguardTeam/AdGuardHome/issues/2688



## [v0.105.1] - 2021-02-15

### Changed
Expand Down
12 changes: 10 additions & 2 deletions internal/dhcpd/dhcpd.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,14 +293,22 @@ func parseOptionString(s string) (uint8, []byte) {
if err != nil {
return 0, nil
}

case "ip":
ip := net.ParseIP(sval)
if ip == nil {
return 0, nil
}
val = ip

// Most DHCP options require IPv4, so do not put the 16-byte
// version if we can. Otherwise, the clients will receive weird
// data that looks like four IPv4 addresses.
//
// See https://github.com/AdguardTeam/AdGuardHome/issues/2688.
if ip4 := ip.To4(); ip4 != nil {
val = ip4
} else {
val = ip
}
default:
return 0, nil
}
Expand Down
34 changes: 24 additions & 10 deletions internal/dhcpd/dhcpd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
package dhcpd

import (
"bytes"
"net"
"os"
"testing"
Expand Down Expand Up @@ -130,46 +129,61 @@ func TestOptions(t *testing.T) {
testCases := []struct {
name string
optStr string
wantCode uint8
wantVal []byte
wantCode uint8
}{{
name: "all_right_hex",
optStr: " 12 hex abcdef ",
wantCode: 12,
name: "success_hex",
optStr: "12 hex abcdef",
wantVal: []byte{0xab, 0xcd, 0xef},
wantCode: 12,
}, {
name: "bad_hex",
optStr: " 12 hex abcdef1 ",
optStr: "12 hex abcdefx",
wantVal: nil,
wantCode: 0,
}, {
name: "all_right_ip",
name: "success_ip",
optStr: "123 ip 1.2.3.4",
wantVal: net.IP{1, 2, 3, 4},
wantCode: 123,
}, {
name: "success_ipv6",
optStr: "123 ip ::1234",
wantVal: net.IP{
0, 0, 0, 0,
0, 0, 0, 0,
0, 0, 0, 0,
0, 0, 0x12, 0x34,
},
wantCode: 123,
wantVal: net.IPv4(1, 2, 3, 4),
}, {
name: "bad_code",
optStr: "256 ip 1.1.1.1",
wantVal: nil,
wantCode: 0,
}, {
name: "negative_code",
optStr: "-1 ip 1.1.1.1",
wantVal: nil,
wantCode: 0,
}, {
name: "bad_ip",
optStr: "12 ip 1.1.1.1x",
wantVal: nil,
wantCode: 0,
}, {
name: "bad_mode",
wantVal: nil,
optStr: "12 x 1.1.1.1",
wantCode: 0,
}}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
code, val := parseOptionString(tc.optStr)
require.EqualValues(t, tc.wantCode, code)
require.Equal(t, tc.wantCode, code)
if tc.wantVal != nil {
assert.True(t, bytes.Equal(tc.wantVal, val))
assert.Equal(t, tc.wantVal, val)
}
})
}
Expand Down

0 comments on commit e17e003

Please sign in to comment.