jenkins-bot has submitted this change and it was merged.
Change subject: Better symlink race protection
......................................................................
Better symlink race protection
We want to support symlinks so tools can put their service
manifest in their git repo and put it anywhere, but also want
to make sure that they don't allow us to read files that belong
to someone else. This is a compromise fix that requires files
to be owned by the tool's uid that should work, I guess.
Bug: T95210
Change-Id: Icb9bbbcf27d555af00b7bca798ac43c7af45d91c
---
M destiny/collector.py
1 file changed, 6 insertions(+), 8 deletions(-)
Approvals:
Yuvipanda: Looks good to me, approved
coren: Looks good to me, but someone else must approve
jenkins-bot: Verified
diff --git a/destiny/collector.py b/destiny/collector.py
index bde1832..55ff02e 100644
--- a/destiny/collector.py
+++ b/destiny/collector.py
@@ -33,16 +33,14 @@
fileparts = manifest_file.split('/')
toolname = fileparts[3] # FIXME: Have extra validation to make
sure this *is* a tool
- # protect against symlink attacks - support symlinks but only if
target and source have same owner
- if os.path.islink(manifest_file):
- realpath = os.path.realpath(manifest_file)
- if os.stat(realpath).st_uid != os.stat(manifest_file).st_uid:
- # Something is amiss, error and don't process this!
- self.log.warn("Ignoring manifest for tool %s, suspicious
symlink", toolname)
- continue
-
with open(manifest_file) as f:
tool = Tool.from_name(toolname)
+ # Support files only if the owner of the file is the tool
itself
+ # This should be ok protection against symlinks to random
places, I think
+ if os.fstat(f.fileno()).st_uid != tool.uid:
+ # Something is amiss, error and don't process this!
+ self.log.warn("Ignoring manifest for tool %s, suspicious
ownership", toolname)
+ continue
manifest = Manifest(tool, yaml.safe_load(f))
manifests.append(manifest)
self.manifests = manifests
--
To view, visit https://gerrit.wikimedia.org/r/202297
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Icb9bbbcf27d555af00b7bca798ac43c7af45d91c
Gerrit-PatchSet: 2
Gerrit-Project: operations/software/tools-manifest
Gerrit-Branch: master
Gerrit-Owner: Yuvipanda <[email protected]>
Gerrit-Reviewer: Legoktm <[email protected]>
Gerrit-Reviewer: Tim Landscheidt <[email protected]>
Gerrit-Reviewer: Yuvipanda <[email protected]>
Gerrit-Reviewer: coren <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
