Skip to content

Fixed a bug that caused template updates to fail #6374

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
+9 −1
Open

Fixed a bug that caused template updates to fail #6374

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion pkg/installer/template.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,9 @@ func (t *TemplateManager) getAbsoluteFilePath(templateDir, uri string, f fs.File

newPath := filepath.Clean(filepath.Join(templateDir, relPath))

if !strings.HasPrefix(newPath, templateDir) {
// Use filepath.Rel for more robust path comparison
relPathToTemplateDir, err := filepath.Rel(templateDir, newPath)
if err != nil || strings.HasPrefix(relPathToTemplateDir, "..") {
// we don't allow LFI
Comment on lines +238 to 241
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Good hardening; make the '..' check segment-aware to avoid false positives.

Using filepath.Rel is the right fix. Replace the plain prefix check with a segment-aware one so names like "..." don’t get incorrectly rejected.

-	relPathToTemplateDir, err := filepath.Rel(templateDir, newPath)
-	if err != nil || strings.HasPrefix(relPathToTemplateDir, "..") {
+	relPathToTemplateDir, err := filepath.Rel(templateDir, newPath)
+	if err != nil {
+		return ""
+	}
+	// Disallow any path that escapes templateDir (first segment is "..")
+	if relPathToTemplateDir == ".." || strings.HasPrefix(relPathToTemplateDir, ".."+string(os.PathSeparator)) {
 		// we don't allow LFI
 		return ""
 	}
🤖 Prompt for AI Agents 
In pkg/installer/template.go around lines 238 to 241, the current check for
directory traversal uses strings.HasPrefix to detect ".." which can cause false
positives with names like "...". Replace this with a segment-aware check by
splitting relPathToTemplateDir using filepath separators and verifying if any
segment equals "..". This ensures only actual directory traversal attempts are
blocked without rejecting valid paths.

return ""
}
Expand Down Expand Up @@ -277,6 +279,12 @@ func (t *TemplateManager) writeTemplatesToDisk(ghrd *updateutils.GHReleaseDownlo
// if error occurs, iteration also stops
return errorutil.NewWithErr(err).Msgf("failed to read file %s", uri)
}

// Ensure the directory exists before writing the file
if err := fileutil.CreateFolder(filepath.Dir(writePath)); err != nil {
return errorutil.NewWithErr(err).Msgf("failed to create directory for %s", writePath)
}

// TODO: It might be better to just download index file from nuclei templates repo
// instead of creating it from scratch
id, _ := config.GetTemplateIDFromReader(bytes.NewReader(bin), uri)
Expand Down
Loading