From c82e93cfc7cf439a5e5bf9b8bc27f5b68b528b53 Mon Sep 17 00:00:00 2001 From: Andrey Meshkov Date: Tue, 20 Aug 2019 00:55:32 +0300 Subject: [PATCH] -(dnsforward): fixed sigsegv when protection is disabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also, fixed all golint issues ✅ Closes: #941 --- .golangci.yml | 1 + dhcpd/check_other_dhcp.go | 4 +--- dnsfilter/dnsfilter.go | 6 +++--- dnsforward/dnsforward.go | 3 +-- dnsforward/dnsforward_test.go | 33 +++++++++++++++++++++++++++++++++ home/home.go | 1 + 6 files changed, 40 insertions(+), 8 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 5dbaf961..ee952d7b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -13,6 +13,7 @@ run: skip-files: - ".*generated.*" - dnsfilter/rule_to_regexp.go + - ".*_test.go" # all available settings of specific linters diff --git a/dhcpd/check_other_dhcp.go b/dhcpd/check_other_dhcp.go index c40e771d..fb7fe98c 100644 --- a/dhcpd/check_other_dhcp.go +++ b/dhcpd/check_other_dhcp.go @@ -17,6 +17,7 @@ import ( // CheckIfOtherDHCPServersPresent sends a DHCP request to the specified network interface, // and waits for a response for a period defined by defaultDiscoverTime +// nolint func CheckIfOtherDHCPServersPresent(ifaceName string) (bool, error) { iface, err := net.InterfaceByName(ifaceName) if err != nil { @@ -95,8 +96,6 @@ func CheckIfOtherDHCPServersPresent(ifaceName string) (bool, error) { if c != nil { defer c.Close() } - // spew.Dump(c, err) - // spew.Printf("net.ListenUDP returned %v, %v\n", c, err) if err != nil { return false, wrapErrPrint(err, "Couldn't listen on :68") } @@ -104,7 +103,6 @@ func CheckIfOtherDHCPServersPresent(ifaceName string) (bool, error) { // send to 255.255.255.255:67 cm := ipv4.ControlMessage{} _, err = c.WriteTo(packet, &cm, dstAddr) - // spew.Dump(n, err) if err != nil { return false, wrapErrPrint(err, "Couldn't send a packet to %s", dst) } diff --git a/dnsfilter/dnsfilter.go b/dnsfilter/dnsfilter.go index 3ca639a3..440170a4 100644 --- a/dnsfilter/dnsfilter.go +++ b/dnsfilter/dnsfilter.go @@ -148,7 +148,7 @@ const ( ReasonRewrite ) -func (i Reason) String() string { +func (r Reason) String() string { names := []string{ "NotFilteredNotFound", "NotFilteredWhiteList", @@ -163,10 +163,10 @@ func (i Reason) String() string { "Rewrite", } - if uint(i) >= uint(len(names)) { + if uint(r) >= uint(len(names)) { return "" } - return names[i] + return names[r] } type dnsFilterContext struct { diff --git a/dnsforward/dnsforward.go b/dnsforward/dnsforward.go index a5aa81fd..0f587cbb 100644 --- a/dnsforward/dnsforward.go +++ b/dnsforward/dnsforward.go @@ -473,7 +473,6 @@ func (s *Server) handleDNSRequest(p *proxy.Proxy, d *proxy.DNSContext) error { } if res.Reason == dnsfilter.ReasonRewrite && len(res.CanonName) != 0 { - d.Req.Question[0] = originalQuestion d.Res.Question[0] = originalQuestion @@ -520,7 +519,7 @@ func (s *Server) filterDNSRequest(d *proxy.DNSContext) (*dnsfilter.Result, error s.RUnlock() if !protectionEnabled { - return nil, nil + return &dnsfilter.Result{}, nil } if host != origHost { diff --git a/dnsforward/dnsforward_test.go b/dnsforward/dnsforward_test.go index 6b685510..ffcb4d67 100644 --- a/dnsforward/dnsforward_test.go +++ b/dnsforward/dnsforward_test.go @@ -78,6 +78,39 @@ func TestServer(t *testing.T) { } } +func TestServerWithProtectionDisabled(t *testing.T) { + s := createTestServer(t) + s.conf.ProtectionEnabled = false + defer removeDataDir(t) + err := s.Start(nil) + if err != nil { + t.Fatalf("Failed to start server: %s", err) + } + + // message over UDP + req := createGoogleATestMessage() + addr := s.dnsProxy.Addr(proxy.ProtoUDP) + client := dns.Client{Net: "udp"} + reply, _, err := client.Exchange(req, addr.String()) + if err != nil { + t.Fatalf("Couldn't talk to server %s: %s", addr, err) + } + assertGoogleAResponse(t, reply) + + // check query log and stats + log := s.GetQueryLog() + assert.Equal(t, 1, len(log), "Log size") + stats := s.GetStatsTop() + assert.Equal(t, 1, len(stats.Domains), "Top domains length") + assert.Equal(t, 0, len(stats.Blocked), "Top blocked length") + assert.Equal(t, 1, len(stats.Clients), "Top clients length") + + err = s.Stop() + if err != nil { + t.Fatalf("DNS server failed to stop: %s", err) + } +} + func TestDotServer(t *testing.T) { // Prepare the proxy server _, certPem, keyPem := createServerTLSConfig(t) diff --git a/home/home.go b/home/home.go index 3064ad80..54a3b0a5 100644 --- a/home/home.go +++ b/home/home.go @@ -61,6 +61,7 @@ func Main(version string, channel string) { // run initializes configuration and runs the AdGuard Home // run is a blocking method and it won't exit until the service is stopped! +// nolint func run(args options) { // config file path can be overridden by command-line arguments: if args.configFilename != "" {