mirrors
/
forgejo
mirror of
1
Fork 0
Code Issues Releases Activity

Fix wiki vulnerabilities 

- Arbitrary file creation leading to command execution
- .md file creation/deletion

Reported by Gabriel Campana.
This commit is contained in:
Unknwon 2016-07-01 15:33:35 +08:00
parent a10ca2c5f6
commit 3a30c06345
1 changed files with 13 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
models/wiki.go
View File

@ -69,10 +69,12 @@ func ToWikiPageURL(name string) string {
return url.QueryEscape(strings.Replace(name, " ", "-", -1)) return url.QueryEscape(strings.Replace(name, " ", "-", -1))
} }
// ToWikiPageName formats a URL back to corresponding wiki page name. // ToWikiPageName formats a URL back to corresponding wiki page name,
// and removes leading characters './' to prevent changing files
// that are not belong to wiki repository.
func ToWikiPageName(urlString string) string { func ToWikiPageName(urlString string) string {
name, _ := url.QueryUnescape(strings.Replace(urlString, "-", " ", -1)) name, _ := url.QueryUnescape(strings.Replace(urlString, "-", " ", -1))
return name return strings.Replace(strings.TrimLeft(name, "./"), "/", " ", -1)
} }
// WikiCloneLink returns clone URLs of repository wiki. // WikiCloneLink returns clone URLs of repository wiki.
@ -149,7 +151,7 @@ func (repo *Repository) updateWikiPage(doer *User, oldTitle, title, content, mes
return fmt.Errorf("UpdateLocalWiki: %v", err) return fmt.Errorf("UpdateLocalWiki: %v", err)
} }
title = ToWikiPageName(strings.Replace(title, "/", " ", -1)) title = ToWikiPageName(title)
filename := path.Join(localPath, title+".md") filename := path.Join(localPath, title+".md")
// If not a new file, show perform update not create. // If not a new file, show perform update not create.
@ -161,6 +163,13 @@ func (repo *Repository) updateWikiPage(doer *User, oldTitle, title, content, mes
os.Remove(path.Join(localPath, oldTitle+".md")) os.Remove(path.Join(localPath, oldTitle+".md"))
} }
// SECURITY: if new file is a symlink to non-exist critical file,
// attack content can be written to the target file (e.g. authorized_keys2)
// as a new page operation.
// So we want to make sure the symlink is removed before write anything.
// The new file we created will be in normal text format.
os.Remove(filename)
if err = ioutil.WriteFile(filename, []byte(content), 0666); err != nil { if err = ioutil.WriteFile(filename, []byte(content), 0666); err != nil {
return fmt.Errorf("WriteFile: %v", err) return fmt.Errorf("WriteFile: %v", err)
} }
@ -198,7 +207,7 @@ func (repo *Repository) DeleteWikiPage(doer *User, title string) (err error) {
return fmt.Errorf("UpdateLocalWiki: %v", err) return fmt.Errorf("UpdateLocalWiki: %v", err)
} }
title = ToWikiPageName(strings.Replace(title, "/", " ", -1)) title = ToWikiPageName(title)
filename := path.Join(localPath, title+".md") filename := path.Join(localPath, title+".md")
os.Remove(filename) os.Remove(filename)