From 249a3a16150891989685f8ca4b23e83a0bfe41f2 Mon Sep 17 00:00:00 2001 From: Fedor Setrakov Date: Fri, 22 May 2026 13:22:59 +0000 Subject: [PATCH] Pull request 2656: AG-54304-path-traversal-vuln Squashed commit of the following: commit 33991debf6893bbfef320be5363bc83e11859039 Merge: 3f88261a5 83d8d6595 Author: f.setrakov Date: Fri May 22 16:10:53 2026 +0300 Merge branch 'master' into AG-54304-path-traversal-vuln commit 3f88261a5bba60243c8fd25fcf898789765e80eb Author: f.setrakov Date: Thu May 21 17:13:37 2026 +0300 home: imp code commit 589589f4f241bcdb5d0c94b5837299b8685443f2 Author: f.setrakov Date: Wed May 20 15:42:34 2026 +0300 all: fix race, imp code commit 762f073facc451e6bc160a21e068352b14806620 Author: f.setrakov Date: Tue May 19 18:13:24 2026 +0300 all: fix path-traversal vuln --- CHANGELOG.md | 2 + internal/home/auth.go | 42 ++++++++++------- internal/home/authglinet.go | 47 +++++++++--------- internal/home/authglinet_internal_test.go | 48 +++++++++++++++---- internal/home/authhttp_internal_test.go | 24 ++++++---- internal/home/home.go | 52 ++++++++++++++------ internal/home/home_internal_test.go | 3 -- internal/home/profilehttp_internal_test.go | 12 ----- internal/home/service.go | 55 +++++++++++++--------- 9 files changed, 175 insertions(+), 110 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8efad6dbcba..9a22fcbdbf6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,8 @@ See also the [v0.107.75 GitHub milestone][ms-v0.107.75]. ### Security +- Authorization in GLiNET mode is no longer vulnerable to path traversal attacks. + - Go version has been updated to prevent the possibility of exploiting the Go vulnerabilities fixed in [1.26.3][go-1.26.3]. - IDs of requests received over DoH and DoQ and forwarded to plain-DNS upstreams are now set to non-zero values to improve security. diff --git a/internal/home/auth.go b/internal/home/auth.go index 3c79c6b3d45..74a6b9dd4f8 100644 --- a/internal/home/auth.go +++ b/internal/home/auth.go @@ -5,6 +5,7 @@ import ( "fmt" "log/slog" "net/http" + "os" "time" "github.com/AdguardTeam/AdGuardHome/internal/aghuser" @@ -56,6 +57,10 @@ type authConfig struct { // mux is the server's multiplexer. It must not be nil. mux *http.ServeMux + // gliNetTokenRoot is the root where GLiNet tokens are stored. It must not + // be nil if isGLiNet is true. + gliNetTokenRoot *os.Root + // rateLimiter manages the rate limiting for login attempts. It must not be // nil. rateLimiter loginRateLimiter @@ -88,6 +93,10 @@ type auth struct { // mux is the server's multiplexer. mux *http.ServeMux + // gliNetTokenRoot is the root where GLiNet tokens are stored. It must not + // be nil if isGLiNet is true. + gliNetTokenRoot *os.Root + // rateLimiter manages rate limiting for login attempts. rateLimiter loginRateLimiter @@ -133,15 +142,16 @@ func newAuth(ctx context.Context, conf *authConfig) (a *auth, err error) { } return &auth{ - logger: conf.baseLogger.With(slogutil.KeyPrefix, "auth"), - mux: conf.mux, - rateLimiter: conf.rateLimiter, - trustedProxies: conf.trustedProxies, - sessions: s, - users: userDB, - doHRoutes: conf.doHRoutes, - isGLiNet: conf.isGLiNet, - isUserless: len(conf.users) == 0, + logger: conf.baseLogger.With(slogutil.KeyPrefix, "auth"), + mux: conf.mux, + rateLimiter: conf.rateLimiter, + trustedProxies: conf.trustedProxies, + gliNetTokenRoot: conf.gliNetTokenRoot, + sessions: s, + users: userDB, + doHRoutes: conf.doHRoutes, + isGLiNet: conf.isGLiNet, + isUserless: len(conf.users) == 0, }, nil } @@ -149,13 +159,13 @@ func newAuth(ctx context.Context, conf *authConfig) (a *auth, err error) { func (a *auth) middleware() (mw httputil.Middleware) { if a.isGLiNet { return newAuthMiddlewareGLiNet(&authMiddlewareGLiNetConfig{ - logger: a.logger, - mux: a.mux, - clock: timeutil.SystemClock{}, - doHRoutes: a.doHRoutes, - tokenFilePrefix: glFilePrefix, - ttl: glTokenTimeout, - maxTokenSize: MaxFileSize, + logger: a.logger, + mux: a.mux, + clock: timeutil.SystemClock{}, + doHRoutes: a.doHRoutes, + tokenFileRoot: a.gliNetTokenRoot, + ttl: glTokenTimeout, + maxTokenSize: MaxFileSize, }) } diff --git a/internal/home/authglinet.go b/internal/home/authglinet.go index f87923e18a2..a96ec9aecd6 100644 --- a/internal/home/authglinet.go +++ b/internal/home/authglinet.go @@ -18,11 +18,9 @@ import ( "github.com/AdguardTeam/golibs/timeutil" ) -// glFilePrefix is the prefix of the filepath where the authentication token is -// stored. Note that it is variable so it can be edited in tests. -// -// TODO(s.chzhen): Make it a constant. -var glFilePrefix = "/tmp/gl_token_" +// glFilePrefix is the prefix of the file where the authentication token is +// stored. +const glFilePrefix = "gl_token_" const ( // glTokenTimeout is the TTL (Time To Live) of the authentication token. @@ -54,9 +52,9 @@ type authMiddlewareGLiNetConfig struct { // doHRoutes is a list of DoH routes for public access. doHRoutes []string - // tokenFilePrefix is the prefix of the filepath where the authentication - // token is stored. It must not be empty. - tokenFilePrefix string + // tokenFileRoot is the root where GLiNet tokens are stored. It must not be + // nil. + tokenFileRoot *os.Root // ttl is the TTL (Time To Live) of the authentication token. It must be // greater than zero. @@ -70,26 +68,26 @@ type authMiddlewareGLiNetConfig struct { // authMiddlewareGLiNet is the GLiNet authentication middleware. It checks if // the request is authenticated using a cookie. type authMiddlewareGLiNet struct { - logger *slog.Logger - mux *http.ServeMux - clock timeutil.Clock - doHRoutes []string - tokenFilePrefix string - ttl time.Duration - maxTokenSize uint + logger *slog.Logger + mux *http.ServeMux + clock timeutil.Clock + doHRoutes []string + tokenFileRoot *os.Root + ttl time.Duration + maxTokenSize uint } // newAuthMiddlewareGLiNet returns the new properly initialized // *authMiddlewareGLiNet. func newAuthMiddlewareGLiNet(c *authMiddlewareGLiNetConfig) (mw *authMiddlewareGLiNet) { return &authMiddlewareGLiNet{ - logger: c.logger, - mux: c.mux, - clock: c.clock, - doHRoutes: c.doHRoutes, - tokenFilePrefix: c.tokenFilePrefix, - ttl: c.ttl, - maxTokenSize: c.maxTokenSize, + logger: c.logger, + mux: c.mux, + clock: c.clock, + doHRoutes: c.doHRoutes, + tokenFileRoot: c.tokenFileRoot, + ttl: c.ttl, + maxTokenSize: c.maxTokenSize, } } @@ -158,8 +156,7 @@ func (mw *authMiddlewareGLiNet) isAuthenticated(ctx context.Context, r *http.Req // the time stored in a file named after the token and checks if the token has // expired based on that time. func (mw *authMiddlewareGLiNet) checkToken(ctx context.Context, token string) (ok bool) { - tokenFile := mw.tokenFilePrefix + token - tokenDate := mw.tokenDate(ctx, tokenFile) + tokenDate := mw.tokenDate(ctx, glFilePrefix+token) now := mw.clock.Now() if now.Before(tokenDate.Add(mw.ttl)) { return true @@ -173,7 +170,7 @@ func (mw *authMiddlewareGLiNet) checkToken(ctx context.Context, token string) (o // tokenDate returns the time stored in the authentication token file. If there // is an error, it logs the error and returns the zero time. func (mw *authMiddlewareGLiNet) tokenDate(ctx context.Context, tokenFile string) (t time.Time) { - f, err := os.Open(tokenFile) + f, err := mw.tokenFileRoot.Open(tokenFile) if err != nil { mw.logger.ErrorContext(ctx, "opening token file", slogutil.KeyError, err) diff --git a/internal/home/authglinet_internal_test.go b/internal/home/authglinet_internal_test.go index c46f50774ac..8b0f3d94623 100644 --- a/internal/home/authglinet_internal_test.go +++ b/internal/home/authglinet_internal_test.go @@ -2,12 +2,15 @@ package home import ( "encoding/binary" + "io/fs" "net/http" "net/http/httptest" "os" + "path/filepath" "testing" "time" + "github.com/AdguardTeam/golibs/testutil" "github.com/AdguardTeam/golibs/timeutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -20,25 +23,42 @@ func TestAuthMiddlewareGLiNet(t *testing.T) { testTTL = 60 * time.Second glTokenFileSuffix = "test" + + testPerm fs.FileMode = 0o644 ) tempDir := t.TempDir() - glFilePrefix = tempDir + "/gl_token_" - glTokenFile := glFilePrefix + glTokenFileSuffix + glTokenFolder := filepath.Join(tempDir, "foo") + err := os.MkdirAll(glTokenFolder, 0o755) + require.NoError(t, err) + + tokenFileRoot, err := os.OpenRoot(glTokenFolder) + require.NoError(t, err) + testutil.CleanupAndRequireSuccess(t, tokenFileRoot.Close) + + err = os.MkdirAll(filepath.Join(glTokenFolder, glFilePrefix), testPerm) + require.NoError(t, err) + + glTokenFile := filepath.Join(glTokenFolder, glFilePrefix+glTokenFileSuffix) glFileData := make([]byte, 4) binary.NativeEndian.PutUint32(glFileData, uint32(time.Now().Add(testTTL).Unix())) - err := os.WriteFile(glTokenFile, glFileData, 0o644) + err = os.WriteFile(glTokenFile, glFileData, testPerm) + require.NoError(t, err) + + // Mock token file for testing path traversal vulnerability. See AG-54304. + passwdFile := filepath.Join(tempDir, "path_traversal_token") + err = os.WriteFile(passwdFile, glFileData, testPerm) require.NoError(t, err) mw := newAuthMiddlewareGLiNet(&authMiddlewareGLiNetConfig{ - logger: testLogger, - mux: http.NewServeMux(), - clock: timeutil.SystemClock{}, - tokenFilePrefix: glFilePrefix, - maxTokenSize: MaxFileSize, - ttl: testTTL, + logger: testLogger, + mux: http.NewServeMux(), + clock: timeutil.SystemClock{}, + tokenFileRoot: tokenFileRoot, + maxTokenSize: MaxFileSize, + ttl: testTTL, }) h := &testAuthHandler{} @@ -50,6 +70,12 @@ func TestAuthMiddlewareGLiNet(t *testing.T) { reqInvalidCookie := httptest.NewRequest(http.MethodGet, "/", nil) reqInvalidCookie.AddCookie(&http.Cookie{Name: glCookieName, Value: "invalid_cookie"}) + reqPathTraversalToken := httptest.NewRequest(http.MethodGet, "/", nil) + reqPathTraversalToken.AddCookie(&http.Cookie{ + Name: glCookieName, + Value: "/../../path_traversal_token", + }) + testCases := []struct { req *http.Request name string @@ -66,6 +92,10 @@ func TestAuthMiddlewareGLiNet(t *testing.T) { req: reqInvalidCookie, name: "invalid_cookie", wantCode: http.StatusFound, + }, { + req: reqPathTraversalToken, + name: "path_traversal_token", + wantCode: http.StatusFound, }} for _, tc := range testCases { diff --git a/internal/home/authhttp_internal_test.go b/internal/home/authhttp_internal_test.go index 09ee7ccdb6c..fbe91d48271 100644 --- a/internal/home/authhttp_internal_test.go +++ b/internal/home/authhttp_internal_test.go @@ -517,6 +517,10 @@ func TestAuth_ServeHTTP_auth(t *testing.T) { writeGLFile(t, tempDir, testTTL) sessionsDB := filepath.Join(tempDir, "sessions.db") + gliNetRoot, err := os.OpenRoot(tempDir) + require.NoError(t, err) + testutil.CleanupAndRequireSuccess(t, gliNetRoot.Close) + mw := &webMw{} baseMux := http.NewServeMux() httpReg := aghhttp.NewDefaultRegistrar(baseMux, mw.wrap) @@ -529,14 +533,15 @@ func TestAuth_ServeHTTP_auth(t *testing.T) { require.NoError(t, err) auth, err := newAuth(testutil.ContextWithTimeout(t, testTimeout), &authConfig{ - baseLogger: testLogger, - mux: baseMux, - rateLimiter: emptyRateLimiter{}, - trustedProxies: testTrustedProxies, - dbFilename: sessionsDB, - users: users, - sessionTTL: testTTL * time.Second, - isGLiNet: false, + baseLogger: testLogger, + mux: baseMux, + rateLimiter: emptyRateLimiter{}, + trustedProxies: testTrustedProxies, + gliNetTokenRoot: gliNetRoot, + dbFilename: sessionsDB, + users: users, + sessionTTL: testTTL * time.Second, + isGLiNet: false, }) require.NoError(t, err) @@ -620,8 +625,7 @@ func TestAuth_ServeHTTP_auth(t *testing.T) { func writeGLFile(t *testing.T, tempDir string, testTTL int64) { t.Helper() - glFilePrefix = tempDir + "/gl_token_" - glTokenFile := glFilePrefix + "test" + glTokenFile := filepath.Join(tempDir, glFilePrefix+"test") glFileData := make([]byte, 4) binary.NativeEndian.PutUint32(glFileData, uint32(time.Now().Unix()+testTTL)) diff --git a/internal/home/home.go b/internal/home/home.go index b99caf0b996..6241cee54e5 100644 --- a/internal/home/home.go +++ b/internal/home/home.go @@ -119,6 +119,12 @@ func Main(clientBuildFS fs.FS) { baseLogger.InfoContext(ctx, "adguard home is running as a service") } + var glTokenFileRoot *os.Root + if opts.glinetMode { + glTokenFileRoot, err = os.OpenRoot("/tmp/") + fatalOnError(err) + } + done := make(chan struct{}) signals := make(chan os.Signal, 1) @@ -126,9 +132,20 @@ func Main(clientBuildFS fs.FS) { sigHdlrLogger := baseLogger.With(slogutil.KeyPrefix, "signalhdlr") sigHdlr := newSignalHandler(sigHdlrLogger, signals, func(ctx context.Context) { + defer close(done) + cleanup(ctx) cleanupAlways() - close(done) + + if !opts.glinetMode { + return + } + + closeErr := glTokenFileRoot.Close() + if closeErr != nil { + baseLogger.ErrorContext(ctx, "closing glinet token root", slogutil.KeyError, closeErr) + os.Exit(osutil.ExitCodeFailure) + } }) go sigHdlr.handle(ctx) @@ -139,6 +156,7 @@ func Main(clientBuildFS fs.FS) { ctx, baseLogger, svcLogger, + glTokenFileRoot, opts, clientBuildFS, signals, @@ -156,7 +174,7 @@ func Main(clientBuildFS fs.FS) { } // run the protection - run(ctx, baseLogger, opts, clientBuildFS, done, sigHdlr, workDir, confPath) + run(ctx, baseLogger, opts, clientBuildFS, glTokenFileRoot, done, sigHdlr, workDir, confPath) } // setupContext initializes [globalContext] fields. It also reads and upgrades @@ -731,7 +749,9 @@ func fatalOnError(err error) { } } -// run configures and starts AdGuard Home. +// run configures and starts AdGuard Home. base and sigHdlr must not be nil. +// glTokenFileRoot must not be nil if opts.glinetMode is true. clientBuildFS +// must not be nil if opts.localFrontend is false. // // TODO(e.burkov): Make opts a pointer. func run( @@ -739,6 +759,7 @@ func run( baseLogger *slog.Logger, opts options, clientBuildFS fs.FS, + glTokenFileRoot *os.Root, done chan struct{}, sigHdlr *signalHandler, workDir string, @@ -796,7 +817,7 @@ func run( err = os.MkdirAll(dataDirPath, aghos.DefaultPermDir) fatalOnError(errors.Annotate(err, "creating DNS data dir at %s: %w", dataDirPath)) - auth, err := initUsers(ctx, baseLogger, workDir, mux, opts.glinetMode) + auth, err := initUsers(ctx, baseLogger, workDir, mux, opts.glinetMode, glTokenFileRoot) fatalOnError(err) confModifier.setAuth(auth) @@ -1040,13 +1061,15 @@ func checkPermissions( } // initUsers initializes authentication module and clears the [config.Users] -// field. +// field. baseLogger and mux must not be nil. glTokenRoot must not be nil if +// isGLiNet is true. func initUsers( ctx context.Context, baseLogger *slog.Logger, workDir string, mux *http.ServeMux, isGLiNet bool, + glTokenRoot *os.Root, ) (auth *auth, err error) { var rateLimiter loginRateLimiter if config.AuthAttempts > 0 && config.AuthBlockMin > 0 { @@ -1059,15 +1082,16 @@ func initUsers( dataDirPath := filepath.Join(workDir, dataDir) auth, err = newAuth(ctx, &authConfig{ - baseLogger: baseLogger, - mux: mux, - rateLimiter: rateLimiter, - trustedProxies: netutil.SliceSubnetSet(netutil.UnembedPrefixes(config.DNS.TrustedProxies)), - dbFilename: filepath.Join(dataDirPath, sessionsDBName), - doHRoutes: config.HTTPConfig.DoH.Routes, - users: config.Users, - sessionTTL: time.Duration(config.HTTPConfig.SessionTTL), - isGLiNet: isGLiNet, + baseLogger: baseLogger, + mux: mux, + rateLimiter: rateLimiter, + trustedProxies: netutil.SliceSubnetSet(netutil.UnembedPrefixes(config.DNS.TrustedProxies)), + dbFilename: filepath.Join(dataDirPath, sessionsDBName), + doHRoutes: config.HTTPConfig.DoH.Routes, + users: config.Users, + sessionTTL: time.Duration(config.HTTPConfig.SessionTTL), + isGLiNet: isGLiNet, + gliNetTokenRoot: glTokenRoot, }) if err != nil { return nil, fmt.Errorf("initializing auth module: %w", err) diff --git a/internal/home/home_internal_test.go b/internal/home/home_internal_test.go index e4c172d6a59..0c3f0a2e0ab 100644 --- a/internal/home/home_internal_test.go +++ b/internal/home/home_internal_test.go @@ -61,7 +61,6 @@ func newTestWeb( // // The global variables are: // - [config] -// - [glFilePrefix] // - [globalContext.clients.storage] // - [globalContext.dnsServer] // - [globalContext.web] @@ -72,14 +71,12 @@ func storeGlobals(tb testing.TB) { tb.Helper() prevConfig := config - prefGLFilePrefix := glFilePrefix storage := globalContext.clients.storage dnsServer := globalContext.dnsServer web := globalContext.web tb.Cleanup(func() { config = prevConfig - glFilePrefix = prefGLFilePrefix globalContext.clients.storage = storage globalContext.dnsServer = dnsServer globalContext.web = web diff --git a/internal/home/profilehttp_internal_test.go b/internal/home/profilehttp_internal_test.go index f5ea2c47e05..209ffa7e0c6 100644 --- a/internal/home/profilehttp_internal_test.go +++ b/internal/home/profilehttp_internal_test.go @@ -3,12 +3,10 @@ package home import ( "bytes" "context" - "encoding/binary" "encoding/json" "io" "net/http" "net/http/httptest" - "os" "path/filepath" "testing" "time" @@ -31,8 +29,6 @@ func TestWeb_HandleGetProfile(t *testing.T) { const ( testTTL = 60 - glTokenFileSuffix = "test" - userName = "name" userPassword = "password" @@ -43,14 +39,6 @@ func TestWeb_HandleGetProfile(t *testing.T) { require.NoError(t, err) tempDir := t.TempDir() - glFilePrefix = tempDir + "/gl_token_" - glTokenFile := glFilePrefix + glTokenFileSuffix - - glFileData := make([]byte, 4) - binary.NativeEndian.PutUint32(glFileData, uint32(time.Now().Unix()+testTTL)) - - err = os.WriteFile(glTokenFile, glFileData, 0o644) - require.NoError(t, err) sessionsDB := filepath.Join(tempDir, "sessions.db") diff --git a/internal/home/service.go b/internal/home/service.go index 3bdfc9d47a0..3bf7e56fd54 100644 --- a/internal/home/service.go +++ b/internal/home/service.go @@ -33,16 +33,17 @@ const svcLogPrefix = "service_manager" // constructing a service instance and running it. Perhaps, deprecate the // action. type program struct { - ctx context.Context - clientBuildFS fs.FS - signals chan os.Signal - done chan struct{} - opts options - baseLogger *slog.Logger - logger *slog.Logger - sigHdlr *signalHandler - workDir string - confPath string + ctx context.Context + clientBuildFS fs.FS + signals chan os.Signal + done chan struct{} + opts options + baseLogger *slog.Logger + logger *slog.Logger + sigHdlr *signalHandler + gliNetTokenRoot *os.Root + workDir string + confPath string } // type check @@ -54,7 +55,17 @@ func (p *program) Start(_ service.Service) (err error) { args := p.opts args.runningAsService = true - go run(p.ctx, p.baseLogger, args, p.clientBuildFS, p.done, p.sigHdlr, p.workDir, p.confPath) + go run( + p.ctx, + p.baseLogger, + args, + p.clientBuildFS, + p.gliNetTokenRoot, + p.done, + p.sigHdlr, + p.workDir, + p.confPath, + ) return nil } @@ -140,6 +151,7 @@ func handleServiceControlAction( ctx context.Context, baseLogger *slog.Logger, l *slog.Logger, + gliNetTokenRoot *os.Root, opts options, clientBuildFS fs.FS, signals chan os.Signal, @@ -167,16 +179,17 @@ func handleServiceControlAction( runOpts.serviceControlAction = "run" p := &program{ - ctx: ctx, - clientBuildFS: clientBuildFS, - signals: signals, - done: done, - opts: runOpts, - baseLogger: baseLogger, - logger: baseLogger.With(slogutil.KeyPrefix, "service"), - sigHdlr: sigHdlr, - workDir: workDir, - confPath: confPath, + ctx: ctx, + clientBuildFS: clientBuildFS, + signals: signals, + done: done, + opts: runOpts, + baseLogger: baseLogger, + logger: baseLogger.With(slogutil.KeyPrefix, "service"), + sigHdlr: sigHdlr, + gliNetTokenRoot: gliNetTokenRoot, + workDir: workDir, + confPath: confPath, } return p.handleRun(ctx, baseLogger, runOpts)