[pull] master from AdguardTeam:master

CHANGELOG.md

@@ -18,6 +18,16 @@ See also the [v0.107.78 GitHub milestone][ms-v0.107.78].
 NOTE: Add new changes BELOW THIS COMMENT.
 -->
 
+### Security
+
+- The size of rulelists is limited. This is necessary to prevent a user's machine from becoming overloaded if the filter source misbehaves.
+
+### Changed
+
+#### Configuration changes
+
+- The `filtering` object of the YAML configuration now includes a new property, `max_http_size`, which defines the maximum size of the HTTP request for rulelists. To disable the limitation, set a large size, such as `1 TB`.
+
 ### Fixed
 
 - Blocked services check on the Custom filtering rules page does not work properly without specifying of a client.

internal/filtering/filter.go

@@ -19,6 +19,7 @@ import (
 	"github.com/AdguardTeam/AdGuardHome/internal/filtering/rulelist"
 	"github.com/AdguardTeam/golibs/container"
 	"github.com/AdguardTeam/golibs/errors"
+	"github.com/AdguardTeam/golibs/ioutil"
 	"github.com/AdguardTeam/golibs/logutil/slogutil"
 )
 
@@ -30,7 +31,8 @@ const filterDir = "filters"
 //
 // TODO(e.burkov):  Investigate if the field ordering is important.
 type FilterYAML struct {
-	Enabled     bool
+	Enabled bool
+	// TODO(m.kazantsev):  Refactor.
 	URL         string    // URL or a file path
 	Name        string    `yaml:"name"`
 	RulesCount  int       `yaml:"-"`
@@ -495,32 +497,86 @@ func (d *DNSFilter) update(filter *FilterYAML) (b bool, err error) {
 }
 
 // updateIntl updates the flt rewriting it's actual file.  It returns true if
-// the actual update has been performed.
+// the actual update has been performed.  flt must not be nil.
 func (d *DNSFilter) updateIntl(ctx context.Context, flt *FilterYAML) (ok bool, err error) {
 	d.logger.DebugContext(ctx, "downloading update for filter", "id", flt.ID, "url", flt.URL)
 
 	var res *rulelist.ParseResult
 
 	tmpFile, err := aghrenameio.NewPendingFile(flt.Path(d.conf.DataDir), aghos.DefaultPermFile)
 	if err != nil {
+		// Don't wrap the error because it's informative enough as is.
 		return false, err
 	}
 	defer func() { err = d.finalizeUpdate(ctx, tmpFile, flt, res, err, ok) }()
 
-	r, err := d.reader(flt.URL)
+	if filepath.IsAbs(flt.URL) {
+		// Initialise this variable to avoid any confusion.
+		path := flt.URL
+
+		res, err = d.readFromFile(tmpFile, path)
+	} else {
+		res, err = d.readFromHTTP(tmpFile, flt.URL)
+	}
+
 	if err != nil {
-		// Don't wrap the error since it's informative enough as is.
+		// Don't wrap the error because it's informative enough as is.
 		return false, err
 	}
-	defer func() { err = errors.WithDeferred(err, r.Close()) }()
+
+	return res.Checksum != flt.checksum, nil
+}
+
+// readFromHTTP reads filter data from urlStr via HTTP and parses it into the
+// tmpFile file.  tmpFile must not be nil.  urlStr must be a valid URL.
+func (d *DNSFilter) readFromHTTP(
+	tmpFile aghrenameio.PendingFile,
+	urlStr string,
+) (res *rulelist.ParseResult, err error) {
+	resp, err := d.conf.HTTPClient.Get(urlStr)
+	if err != nil {
+		// Don't wrap the error because it's informative enough as is.
+		return nil, err
+	}
+	defer func() { err = errors.WithDeferred(err, resp.Body.Close()) }()
+
+	if resp.StatusCode != http.StatusOK {
+		return nil, fmt.Errorf("got status code %d, want %d", resp.StatusCode, http.StatusOK)
+	}
+
+	bufPtr := d.bufPool.Get()
+	defer d.bufPool.Put(bufPtr)
+
+	p := rulelist.NewParser()
+	httpBody := ioutil.LimitReader(resp.Body, d.conf.MaxHTTPSize.Bytes())
+
+	return p.Parse(tmpFile, httpBody, *bufPtr)
+}
+
+// readFromFile reads filter data from a file located at path and parses it into
+// the tmpFile file.  tmpFile must not be nil.  path must be a valid filepath.
+func (d *DNSFilter) readFromFile(
+	tmpFile aghrenameio.PendingFile,
+	path string,
+) (res *rulelist.ParseResult, err error) {
+	path = filepath.Clean(path)
+
+	if !pathMatchesAny(d.safeFSPatterns, path) {
+		return nil, fmt.Errorf("path %q does not match safe patterns", path)
+	}
+
+	file, err := os.Open(path)
+	if err != nil {
+		return nil, fmt.Errorf("opening file: %w", err)
+	}
+	defer func() { err = errors.WithDeferred(err, file.Close()) }()
 
 	bufPtr := d.bufPool.Get()
 	defer d.bufPool.Put(bufPtr)
 
 	p := rulelist.NewParser()
-	res, err = p.Parse(tmpFile, r, *bufPtr)
 
-	return res.Checksum != flt.checksum && err == nil, err
+	return p.Parse(tmpFile, file, *bufPtr)
 }
 
 // finalizeUpdate closes and gets rid of temporary file f with filter's content
@@ -566,47 +622,6 @@ func (d *DNSFilter) finalizeUpdate(
 	return nil
 }
 
-// reader returns an io.ReadCloser reading filtering-rule list data form either
-// a file on the filesystem or the filter's HTTP URL.
-func (d *DNSFilter) reader(fltURL string) (r io.ReadCloser, err error) {
-	if !filepath.IsAbs(fltURL) {
-		r, err = d.readerFromURL(fltURL)
-		if err != nil {
-			return nil, fmt.Errorf("reading from url: %w", err)
-		}
-
-		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)
-	}
-
-	return r, nil
-}
-
-// readerFromURL returns an io.ReadCloser reading filtering-rule list data form
-// the filter's URL.
-func (d *DNSFilter) readerFromURL(fltURL string) (r io.ReadCloser, err error) {
-	resp, err := d.conf.HTTPClient.Get(fltURL)
-	if err != nil {
-		// Don't wrap the error since it's informative enough as is.
-		return nil, err
-	}
-
-	if resp.StatusCode != http.StatusOK {
-		return nil, fmt.Errorf("got status code %d, want %d", resp.StatusCode, http.StatusOK)
-	}
-
-	return resp.Body, nil
-}
-
 // loads filter contents from the file in dataDir
 func (d *DNSFilter) load(ctx context.Context, flt *FilterYAML) (err error) {
 	fileName := flt.Path(d.conf.DataDir)

internal/filtering/filter_internal_test.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/AdguardTeam/golibs/netutil/urlutil"
 	"github.com/AdguardTeam/golibs/testutil"
+	"github.com/c2h5oh/datasize"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -76,6 +77,9 @@ func updateAndAssert(
 	require.NoError(tb, err)
 }
 
+// testFilterSize is a test size of filters.
+const testFilterSize = 10 * datasize.MB
+
 // newDNSFilter returns a new properly initialized DNS filter instance.
 func newDNSFilter(tb testing.TB) (d *DNSFilter) {
 	tb.Helper()
@@ -86,6 +90,7 @@ func newDNSFilter(tb testing.TB) (d *DNSFilter) {
 		HTTPClient: &http.Client{
 			Timeout: testTimeout,
 		},
+		MaxHTTPSize: testFilterSize,
 	}, nil)
 	require.NoError(tb, err)
 

internal/filtering/filtering.go

@@ -32,6 +32,7 @@ import (
 	"github.com/AdguardTeam/urlfilter"
 	"github.com/AdguardTeam/urlfilter/filterlist"
 	"github.com/AdguardTeam/urlfilter/rules"
+	"github.com/c2h5oh/datasize"
 	"github.com/miekg/dns"
 )
 
@@ -154,6 +155,10 @@ type Config struct {
 	// files can be added.
 	SafeFSPatterns []string `yaml:"safe_fs_patterns"`
 
+	// MaxHTTPSize defines the maximum size of the HTTP body.  The value must
+	// not be equal to zero.
+	MaxHTTPSize datasize.ByteSize `yaml:"max_http_size"`
+
 	SafeBrowsingCacheSize uint `yaml:"safebrowsing_cache_size"` // (in bytes)
 	SafeSearchCacheSize   uint `yaml:"safesearch_cache_size"`   // (in bytes)
 	ParentalCacheSize     uint `yaml:"parental_cache_size"`     // (in bytes)

internal/filtering/http_internal_test.go

@@ -119,6 +119,7 @@ func TestDNSFilter_handleFilteringSetURL(t *testing.T) {
 				ConfModifier: confModifier,
 				HTTPReg:      aghhttp.EmptyRegistrar{},
 				DataDir:      filtersDir,
+				MaxHTTPSize:  testFilterSize,
 			}, nil)
 			require.NoError(t, err)
 			t.Cleanup(d.Close)

internal/filtering/rulelist/rulelist.go

@@ -21,7 +21,7 @@ import (
 const DefaultRuleBufSize = 1024
 
 // DefaultMaxRuleListSize is the default maximum filtering-rule list size.
-const DefaultMaxRuleListSize = 64 * datasize.MB
+const DefaultMaxRuleListSize = 256 * datasize.MB
 
 // APIID is the type for the rule-list IDs used in the HTTP API.
 type APIID int64

internal/home/config.go

@@ -20,6 +20,7 @@ import (
 	"github.com/AdguardTeam/AdGuardHome/internal/dhcpd"
 	"github.com/AdguardTeam/AdGuardHome/internal/dnsforward"
 	"github.com/AdguardTeam/AdGuardHome/internal/filtering"
+	"github.com/AdguardTeam/AdGuardHome/internal/filtering/rulelist"
 	"github.com/AdguardTeam/AdGuardHome/internal/querylog"
 	"github.com/AdguardTeam/AdGuardHome/internal/schedule"
 	"github.com/AdguardTeam/AdGuardHome/internal/stats"
@@ -564,6 +565,7 @@ var config = &configuration{
 		ParentalEnabled:     false,
 		SafeBrowsingEnabled: false,
 
+		MaxHTTPSize:           rulelist.DefaultMaxRuleListSize,
 		SafeBrowsingCacheSize: 1 * 1024 * 1024,
 		SafeSearchCacheSize:   1 * 1024 * 1024,
 		ParentalCacheSize:     1 * 1024 * 1024,