Hi Chad, Chad Dougherty <[email protected]> writes:
> On 12/10/25 11:18 PM, Alan Coopersmith wrote: >> https://github.com/gogs/gogs offers a MIT-licensed self-hosted git service. >> https://www.wiz.io/blog/wiz-research-gogs-cve-2025-8110-rce-exploit >> warns of >> CVE-2025-8110, an as-yet-unfixed vulnerability in this service which >> they say >> they are seeing being actively exploited. >> > > FYI, this was reportedly fixed in https://github.com/gogs/gogs/pull/8082 Thanks for the link. But I'm not very confident this actually fixes the issue. Copying some select lines of code from that patch: > localPath := r.LocalCopyPath() > [...] > + // 🚨 SECURITY: Prevent touching files in surprising places, reject > operations > + // involves symlinks. > + if hasSymlinkInPath(localPath, opts.OldTreeName) || > hasSymlinkInPath(localPath, opts.NewTreeName) { > + return errors.New("cannot update file with symbolic link in > path") > + } > + > + repoPath := r.RepoPath() > > + newFilePath := path.Join(localPath, opts.NewTreeName) > [...] > + if err := os.MkdirAll(path.Dir(newFilePath), os.ModePerm); err != nil { > + return errors.Wrapf(err, "create parent directories of %q", > newFilePath) > + } Where hasSymlinkInPath() is defined here: > +// hasSymlinkInPath returns true if there is any symlink in path hierarchy > using > +// the given base and relative path. > +func hasSymlinkInPath(base, relPath string) bool { > + parts := strings.Split(filepath.ToSlash(relPath), "/") > + for i := range parts { > + filePath := path.Join(append([]string{base}, parts[:i+1]...)...) > + if osutil.IsSymlink(filePath) { > + return true > + } > + } > + return false > +} This just introduces TOCTOU races, no? If someone can delete a portion of "opts.NewTreeName" and recreate an element as a symbolic link before "os.MkdirAll" is executed, they would be able to achieve the same thing as before the patch. Surely Go has a way to use O_NOFOLLOW, right? That would be the correct way to do it. Collin
