Skip to content

Commit 8b673f7

Browse files
committed
Fix symlink traversal bug
The Bug The tokenPath() function in internal/oauth/oauth.go:301-317 used strings.HasPrefix to verify paths stayed within tokensDir, but didn't detect symlinks. An attacker who could create a symlink inside tokensDir (e.g., tokensDir/evil.json -> /etc/passwd) could cause token data to be written outside the tokens directory. The Fix Added symlink detection using os.Lstat() and filepath.EvalSymlinks(): // Check if path is a symlink that could escape tokensDir if info, err := os.Lstat(cleanPath); err == nil && info.Mode()&os.ModeSymlink != 0 { // Path exists and is a symlink - resolve it and verify it stays within tokensDir resolved, err := filepath.EvalSymlinks(cleanPath) if err != nil || !isPathWithinDir(resolved, cleanTokensDir) { // Symlink resolution failed or escapes tokensDir - use hash-based fallback return filepath.Join(m.tokensDir, fmt.Sprintf("%x.json", sha256.Sum256([]byte(email)))) } } Also added a helper function isPathWithinDir() that properly resolves symlinks in the base directory before comparing paths. Changes Made - internal/oauth/oauth.go: Added symlink detection in tokenPath() and new isPathWithinDir() helper (13 lines added) - internal/oauth/oauth_test.go: Added TestTokenPath_SymlinkEscape test case (47 lines added)
1 parent 84f254a commit 8b673f7

2 files changed

Lines changed: 77 additions & 1 deletion

File tree

internal/oauth/oauth.go

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -306,16 +306,44 @@ func (m *Manager) tokenPath(email string) string {
306306
// Ensure the final path is within tokensDir
307307
path := filepath.Join(m.tokensDir, safe+".json")
308308
cleanPath := filepath.Clean(path)
309+
cleanTokensDir := filepath.Clean(m.tokensDir)
309310

310311
// Verify the path is still within tokensDir
311-
if !strings.HasPrefix(cleanPath, filepath.Clean(m.tokensDir)) {
312+
if !strings.HasPrefix(cleanPath, cleanTokensDir) {
312313
// If path escapes tokensDir, use a hash-based fallback
313314
return filepath.Join(m.tokensDir, fmt.Sprintf("%x.json", sha256.Sum256([]byte(email))))
314315
}
315316

317+
// Check if path is a symlink that could escape tokensDir
318+
if info, err := os.Lstat(cleanPath); err == nil && info.Mode()&os.ModeSymlink != 0 {
319+
// Path exists and is a symlink - resolve it and verify it stays within tokensDir
320+
resolved, err := filepath.EvalSymlinks(cleanPath)
321+
if err != nil || !isPathWithinDir(resolved, cleanTokensDir) {
322+
// Symlink resolution failed or escapes tokensDir - use hash-based fallback
323+
return filepath.Join(m.tokensDir, fmt.Sprintf("%x.json", sha256.Sum256([]byte(email))))
324+
}
325+
}
326+
316327
return cleanPath
317328
}
318329

330+
// isPathWithinDir checks if path is within dir, resolving symlinks in dir.
331+
func isPathWithinDir(path, dir string) bool {
332+
// Resolve symlinks in dir to get the real base directory
333+
resolvedDir, err := filepath.EvalSymlinks(dir)
334+
if err != nil {
335+
resolvedDir = dir // fallback to original if dir doesn't exist yet
336+
}
337+
resolvedDir = filepath.Clean(resolvedDir)
338+
339+
// Clean and check the path
340+
cleanPath := filepath.Clean(path)
341+
342+
// Ensure path is within dir (with proper separator handling)
343+
return strings.HasPrefix(cleanPath, resolvedDir+string(filepath.Separator)) ||
344+
cleanPath == resolvedDir
345+
}
346+
319347
// scopesToString joins scopes with spaces.
320348
func scopesToString(scopes []string) string {
321349
return strings.Join(scopes, " ")

internal/oauth/oauth_test.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,54 @@ func TestSanitizeEmail(t *testing.T) {
249249
}
250250
}
251251

252+
func TestTokenPath_SymlinkEscape(t *testing.T) {
253+
// This test verifies that symlinks inside tokensDir cannot be used
254+
// to write tokens outside the tokens directory.
255+
//
256+
// Attack scenario:
257+
// 1. Attacker creates symlink: tokensDir/evil.json -> /tmp/outside/evil.json
258+
// 2. saveToken("evil", ...) would follow the symlink and write outside tokensDir
259+
// 3. The fix should detect this and use a hash-based fallback path
260+
261+
dir := t.TempDir()
262+
tokensDir := filepath.Join(dir, "tokens")
263+
outsideDir := filepath.Join(dir, "outside")
264+
265+
if err := os.MkdirAll(tokensDir, 0700); err != nil {
266+
t.Fatal(err)
267+
}
268+
if err := os.MkdirAll(outsideDir, 0700); err != nil {
269+
t.Fatal(err)
270+
}
271+
272+
// Create a symlink inside tokensDir that points outside
273+
symlinkPath := filepath.Join(tokensDir, "evil.json")
274+
outsideTarget := filepath.Join(outsideDir, "evil.json")
275+
if err := os.Symlink(outsideTarget, symlinkPath); err != nil {
276+
t.Skipf("cannot create symlink (may require admin on Windows): %v", err)
277+
}
278+
279+
mgr := &Manager{
280+
config: &oauth2.Config{Scopes: Scopes},
281+
tokensDir: tokensDir,
282+
}
283+
284+
// Get the token path for "evil" - this should NOT return the symlink path
285+
// because following it would write outside tokensDir
286+
gotPath := mgr.tokenPath("evil")
287+
288+
// The path should NOT be the symlink (which would write outside tokensDir)
289+
if gotPath == symlinkPath {
290+
t.Errorf("tokenPath returned symlink path %q, should use hash-based fallback to prevent escape", gotPath)
291+
}
292+
293+
// Verify the returned path, when resolved, stays within tokensDir
294+
// (the hash-based fallback should be used)
295+
if !strings.HasPrefix(gotPath, tokensDir) {
296+
t.Errorf("tokenPath %q is not within tokensDir %q", gotPath, tokensDir)
297+
}
298+
}
299+
252300
func TestParseClientSecrets(t *testing.T) {
253301
// Valid Desktop application credentials
254302
validDesktop := `{

0 commit comments

Comments
 (0)