all: add permcheck, client fix; imp chlog

internal/filtering/filter.go

@@ -11,6 +11,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/AdguardTeam/AdGuardHome/internal/aghos"
 	"github.com/AdguardTeam/AdGuardHome/internal/aghrenameio"
 	"github.com/AdguardTeam/AdGuardHome/internal/filtering/rulelist"
 	"github.com/AdguardTeam/golibs/container"
@@ -448,11 +449,7 @@ func (d *DNSFilter) updateIntl(flt *FilterYAML) (ok bool, err error) {
 
 	var res *rulelist.ParseResult
 
-	// Change the default 0o600 permission to something more acceptable by end
-	// users.
-	//
-	// See https://github.com/AdguardTeam/AdGuardHome/issues/3198.
-	tmpFile, err := aghrenameio.NewPendingFile(flt.Path(d.conf.DataDir), 0o644)
+	tmpFile, err := aghrenameio.NewPendingFile(flt.Path(d.conf.DataDir), aghos.DefaultPermFile)
 	if err != nil {
 		return false, err
 	}
@@ -522,6 +519,11 @@ func (d *DNSFilter) reader(fltURL string) (r io.ReadCloser, err error) {
 		return r, nil
 	}
 
+	fltURL = filepath.Clean(fltURL)
+	if !pathMatchesAny(d.safeFSPatterns, fltURL) {
+		return nil, fmt.Errorf("path %q does not match safe patterns", fltURL)
+	}
+
 	r, err = os.Open(fltURL)
 	if err != nil {
 		return nil, fmt.Errorf("opening file: %w", err)

internal/filtering/filtering.go

@@ -19,6 +19,7 @@ import (
 	"time"
 
 	"github.com/AdguardTeam/AdGuardHome/internal/aghhttp"
+	"github.com/AdguardTeam/AdGuardHome/internal/aghos"
 	"github.com/AdguardTeam/AdGuardHome/internal/filtering/rulelist"
 	"github.com/AdguardTeam/golibs/container"
 	"github.com/AdguardTeam/golibs/errors"
@@ -130,6 +131,10 @@ type Config struct {
 	// UserRules is the global list of custom rules.
 	UserRules []string `yaml:"-"`
 
+	// SafeFSPatterns are the patterns for matching which local filtering-rule
+	// files can be added.
+	SafeFSPatterns []string `yaml:"safe_fs_patterns"`
+
 	SafeBrowsingCacheSize uint `yaml:"safebrowsing_cache_size"` // (in bytes)
 	SafeSearchCacheSize   uint `yaml:"safesearch_cache_size"`   // (in bytes)
 	ParentalCacheSize     uint `yaml:"parental_cache_size"`     // (in bytes)
@@ -257,6 +262,8 @@ type DNSFilter struct {
 	refreshLock *sync.Mutex
 
 	hostCheckers []hostChecker
+
+	safeFSPatterns []string
 }
 
 // Filter represents a filter list
@@ -987,13 +994,22 @@ func New(c *Config, blockFilters []Filter) (d *DNSFilter, err error) {
 	d = &DNSFilter{
 		idGen:                  newIDGenerator(int32(time.Now().Unix())),
 		bufPool:                syncutil.NewSlicePool[byte](rulelist.DefaultRuleBufSize),
+		safeSearch:             c.SafeSearch,
 		refreshLock:            &sync.Mutex{},
 		safeBrowsingChecker:    c.SafeBrowsingChecker,
 		parentalControlChecker: c.ParentalControlChecker,
 		confMu:                 &sync.RWMutex{},
 	}
 
-	d.safeSearch = c.SafeSearch
+	for i, p := range c.SafeFSPatterns {
+		// Use Match to validate the patterns here.
+		_, err = filepath.Match(p, "test")
+		if err != nil {
+			return nil, fmt.Errorf("safe_fs_patterns: at index %d: %w", i, err)
+		}
+
+		d.safeFSPatterns = append(d.safeFSPatterns, p)
+	}
 
 	d.hostCheckers = []hostChecker{{
 		check: d.matchSysHosts,
@@ -1022,7 +1038,7 @@ func New(c *Config, blockFilters []Filter) (d *DNSFilter, err error) {
 
 	err = d.prepareRewrites()
 	if err != nil {
-		return nil, fmt.Errorf("rewrites: preparing: %s", err)
+		return nil, fmt.Errorf("rewrites: preparing: %w", err)
 	}
 
 	if d.conf.BlockedServices != nil {
@@ -1037,11 +1053,16 @@ func New(c *Config, blockFilters []Filter) (d *DNSFilter, err error) {
 		if err != nil {
 			d.Close()
 
-			return nil, fmt.Errorf("initializing filtering subsystem: %s", err)
+			return nil, fmt.Errorf("initializing filtering subsystem: %w", err)
 		}
 	}
 
-	_ = os.MkdirAll(filepath.Join(d.conf.DataDir, filterDir), 0o755)
+	err = os.MkdirAll(filepath.Join(d.conf.DataDir, filterDir), aghos.DefaultPermDir)
+	if err != nil {
+		d.Close()
+
+		return nil, fmt.Errorf("making filtering directory: %w", err)
+	}
 
 	d.loadFilters(d.conf.Filters)
 	d.loadFilters(d.conf.WhitelistFilters)

internal/filtering/http.go

@@ -20,14 +20,22 @@ import (
 )
 
 // validateFilterURL validates the filter list URL or file name.
-func validateFilterURL(urlStr string) (err error) {
+func (d *DNSFilter) validateFilterURL(urlStr string) (err error) {
 	defer func() { err = errors.Annotate(err, "checking filter: %w") }()
 
 	if filepath.IsAbs(urlStr) {
+		urlStr = filepath.Clean(urlStr)
 		_, err = os.Stat(urlStr)
+		if err != nil {
+			// Don't wrap the error since it's informative enough as is.
+			return err
+		}
 
-		// Don't wrap the error since it's informative enough as is.
-		return err
+		if !pathMatchesAny(d.safeFSPatterns, urlStr) {
+			return fmt.Errorf("path %q does not match safe patterns", urlStr)
+		}
+
+		return nil
 	}
 
 	u, err := url.ParseRequestURI(urlStr)
@@ -65,7 +73,7 @@ func (d *DNSFilter) handleFilteringAddURL(w http.ResponseWriter, r *http.Request
 		return
 	}
 
-	err = validateFilterURL(fj.URL)
+	err = d.validateFilterURL(fj.URL)
 	if err != nil {
 		aghhttp.Error(r, w, http.StatusBadRequest, "%s", err)
 
@@ -225,7 +233,7 @@ func (d *DNSFilter) handleFilteringSetURL(w http.ResponseWriter, r *http.Request
 		return
 	}
 
-	err = validateFilterURL(fj.Data.URL)
+	err = d.validateFilterURL(fj.Data.URL)
 	if err != nil {
 		aghhttp.Error(r, w, http.StatusBadRequest, "invalid url: %s", err)
 

internal/filtering/path.go

@@ -0,0 +1,37 @@
+package filtering
+
+import (
+	"fmt"
+	"path/filepath"
+)
+
+// pathMatchesAny returns true if filePath matches one of globs.  globs must be
+// valid.  filePath must be absolute and clean.  If globs are empty,
+// pathMatchesAny returns false.
+//
+// TODO(a.garipov): Move to golibs?
+func pathMatchesAny(globs []string, filePath string) (ok bool) {
+	if len(globs) == 0 {
+		return false
+	}
+
+	clean, err := filepath.Abs(filePath)
+	if err != nil {
+		panic(fmt.Errorf("pathMatchesAny: %w", err))
+	} else if clean != filePath {
+		panic(fmt.Errorf("pathMatchesAny: filepath %q is not absolute", filePath))
+	}
+
+	for _, g := range globs {
+		ok, err = filepath.Match(g, filePath)
+		if err != nil {
+			panic(fmt.Errorf("pathMatchesAny: bad pattern: %w", err))
+		}
+
+		if ok {
+			return true
+		}
+	}
+
+	return false
+}

internal/filtering/path_unix_internal_test.go

@@ -0,0 +1,78 @@
+//go:build unix
+
+package filtering
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestPathInAnyDir(t *testing.T) {
+	t.Parallel()
+
+	const (
+		filePath      = "/path/to/file.txt"
+		filePathGlob  = "/path/to/*"
+		otherFilePath = "/otherpath/to/file.txt"
+	)
+
+	testCases := []struct {
+		want     assert.BoolAssertionFunc
+		filePath string
+		name     string
+		globs    []string
+	}{{
+		want:     assert.False,
+		filePath: filePath,
+		name:     "nil_pats",
+		globs:    nil,
+	}, {
+		want:     assert.True,
+		filePath: filePath,
+		name:     "match",
+		globs: []string{
+			filePath,
+			otherFilePath,
+		},
+	}, {
+		want:     assert.False,
+		filePath: filePath,
+		name:     "no_match",
+		globs: []string{
+			otherFilePath,
+		},
+	}, {
+		want:     assert.True,
+		filePath: filePath,
+		name:     "match_star",
+		globs: []string{
+			filePathGlob,
+		},
+	}}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			tc.want(t, pathMatchesAny(tc.globs, tc.filePath))
+		})
+	}
+
+	require.True(t, t.Run("panic_on_unabs_file_path", func(t *testing.T) {
+		t.Parallel()
+
+		assert.Panics(t, func() {
+			_ = pathMatchesAny([]string{"/home/user"}, "../../etc/passwd")
+		})
+	}))
+
+	require.True(t, t.Run("panic_on_bad_pat", func(t *testing.T) {
+		t.Parallel()
+
+		assert.Panics(t, func() {
+			_ = pathMatchesAny([]string{`\`}, filePath)
+		})
+	}))
+}

internal/filtering/path_windows_internal_test.go

@@ -0,0 +1,73 @@
+//go:build windows
+
+package filtering
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestPathInAnyDir(t *testing.T) {
+	t.Parallel()
+
+	const (
+		filePath      = `C:\path\to\file.txt`
+		filePathGlob  = `C:\path\to\*`
+		otherFilePath = `C:\otherpath\to\file.txt`
+	)
+
+	testCases := []struct {
+		want     assert.BoolAssertionFunc
+		filePath string
+		name     string
+		globs    []string
+	}{{
+		want:     assert.False,
+		filePath: filePath,
+		name:     "nil_pats",
+		globs:    nil,
+	}, {
+		want:     assert.True,
+		filePath: filePath,
+		name:     "match",
+		globs: []string{
+			filePath,
+			otherFilePath,
+		},
+	}, {
+		want:     assert.False,
+		filePath: filePath,
+		name:     "no_match",
+		globs: []string{
+			otherFilePath,
+		},
+	}, {
+		want:     assert.True,
+		filePath: filePath,
+		name:     "match_star",
+		globs: []string{
+			filePathGlob,
+		},
+	}}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			tc.want(t, pathMatchesAny(tc.globs, tc.filePath))
+		})
+	}
+
+	require.True(t, t.Run("panic_on_unabs_file_path", func(t *testing.T) {
+		t.Parallel()
+
+		assert.Panics(t, func() {
+			_ = pathMatchesAny([]string{`C:\home\user`}, `..\..\etc\passwd`)
+		})
+	}))
+
+	// TODO(a.garipov): See if there is anything for which filepath.Match
+	// returns ErrBadPattern on Windows.
+}

internal/filtering/rulelist/filter.go

@@ -11,6 +11,7 @@ import (
 	"path/filepath"
 	"time"
 
+	"github.com/AdguardTeam/AdGuardHome/internal/aghos"
 	"github.com/AdguardTeam/AdGuardHome/internal/aghrenameio"
 	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/ioutil"
@@ -196,7 +197,7 @@ func (f *Filter) readFromHTTP(
 		return "", nil, fmt.Errorf("got status code %d, want %d", resp.StatusCode, http.StatusOK)
 	}
 
-	fltFile, err := aghrenameio.NewPendingFile(cachePath, 0o644)
+	fltFile, err := aghrenameio.NewPendingFile(cachePath, aghos.DefaultPermFile)
 	if err != nil {
 		return "", nil, fmt.Errorf("creating temp file: %w", err)
 	}
@@ -271,7 +272,7 @@ func parseIntoCache(
 	filePath string,
 	cachePath string,
 ) (parseRes *ParseResult, err error) {
-	tmpFile, err := aghrenameio.NewPendingFile(cachePath, 0o644)
+	tmpFile, err := aghrenameio.NewPendingFile(cachePath, aghos.DefaultPermFile)
 	if err != nil {
 		return nil, fmt.Errorf("creating temp file: %w", err)
 	}