From ab46965d9535d53850b2dc960fe2c7a1734eee48 Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Sat, 13 Jul 2024 15:23:39 -0400 Subject: [PATCH 1/4] Deny file mode changes outside of specified paths in sandbox --- Library/Homebrew/sandbox.rb | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Library/Homebrew/sandbox.rb b/Library/Homebrew/sandbox.rb index 0cbd7db525e55..4cf3fade5d1d7 100644 --- a/Library/Homebrew/sandbox.rb +++ b/Library/Homebrew/sandbox.rb @@ -37,6 +37,14 @@ def add_rule(allow:, operation:, filter: nil, modifier: nil) def allow_write(path:, type: :literal) add_rule allow: true, operation: "file-write*", filter: path_filter(path, type) add_rule allow: true, operation: "file-write-setugid", filter: path_filter(path, type) + + file_write_mode_path = if Pathname(path).directory? + "#{path}/*" + else + path + end + + add_rule allow: true, operation: "file-write-mode", filter: path_filter(file_write_mode_path, type) end sig { params(path: T.any(String, Pathname), type: Symbol).void } @@ -289,6 +297,7 @@ class SandboxProfile (regex #"^/dev/tty[a-z0-9]*$") ) (deny file-write*) ; deny non-allowlist file write operations + (deny file-write-mode) ; deny non-allowlist file write mode operations (allow process-exec (literal "/bin/ps") (with no-sandbox) From ea364210f290544b7d03e8d467e3588e15a2d23c Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Sat, 13 Jul 2024 15:58:41 -0400 Subject: [PATCH 2/4] Remove unecessary directory check --- Library/Homebrew/sandbox.rb | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/Library/Homebrew/sandbox.rb b/Library/Homebrew/sandbox.rb index 4cf3fade5d1d7..c4bd39e2604be 100644 --- a/Library/Homebrew/sandbox.rb +++ b/Library/Homebrew/sandbox.rb @@ -37,14 +37,7 @@ def add_rule(allow:, operation:, filter: nil, modifier: nil) def allow_write(path:, type: :literal) add_rule allow: true, operation: "file-write*", filter: path_filter(path, type) add_rule allow: true, operation: "file-write-setugid", filter: path_filter(path, type) - - file_write_mode_path = if Pathname(path).directory? - "#{path}/*" - else - path - end - - add_rule allow: true, operation: "file-write-mode", filter: path_filter(file_write_mode_path, type) + add_rule allow: true, operation: "file-write-mode", filter: path_filter(path, type) end sig { params(path: T.any(String, Pathname), type: Symbol).void } From 74bb9fb193a95979c7815fa9c2ae8d97133e6efe Mon Sep 17 00:00:00 2001 From: Thierry Moisan Date: Sat, 13 Jul 2024 16:07:48 -0400 Subject: [PATCH 3/4] Add test --- Library/Homebrew/test/sandbox_spec.rb | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/Library/Homebrew/test/sandbox_spec.rb b/Library/Homebrew/test/sandbox_spec.rb index 79845beaa2718..4f87e12a03b77 100644 --- a/Library/Homebrew/test/sandbox_spec.rb +++ b/Library/Homebrew/test/sandbox_spec.rb @@ -57,4 +57,18 @@ .and output(a_string_matching(/foo/).and(matching(/bar/).and(not_matching(/Python/)))).to_stdout end end + + describe "#disallow chmod on some directory" do + it "formula does a chmod to opt" do + expect { sandbox.exec "chmod", "ug-w", HOMEBREW_PREFIX}.to raise_error(ErrorDuringExecution) + end + + it "allows chmod on a path allowed to write" do + mktmpdir do |path| + FileUtils.touch path/"foo" + sandbox.allow_write_path(path) + expect { sandbox.exec "chmod", "ug-w", path/"foo"}.not_to raise_error(ErrorDuringExecution) + end + end + end end From e054a3ccf6401e5c6ebc87a147a81bd56e8e3132 Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Sat, 13 Jul 2024 16:28:17 -0400 Subject: [PATCH 4/4] Also restrict SUID/GSID writes in sandbox --- Library/Homebrew/sandbox.rb | 1 + Library/Homebrew/test/sandbox_spec.rb | 18 ++++++++++++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/sandbox.rb b/Library/Homebrew/sandbox.rb index c4bd39e2604be..2a78e2f5cbe67 100644 --- a/Library/Homebrew/sandbox.rb +++ b/Library/Homebrew/sandbox.rb @@ -290,6 +290,7 @@ class SandboxProfile (regex #"^/dev/tty[a-z0-9]*$") ) (deny file-write*) ; deny non-allowlist file write operations + (deny file-write-setugid) ; deny non-allowlist file write SUID/SGID operations (deny file-write-mode) ; deny non-allowlist file write mode operations (allow process-exec (literal "/bin/ps") diff --git a/Library/Homebrew/test/sandbox_spec.rb b/Library/Homebrew/test/sandbox_spec.rb index 4f87e12a03b77..f0954877af393 100644 --- a/Library/Homebrew/test/sandbox_spec.rb +++ b/Library/Homebrew/test/sandbox_spec.rb @@ -60,14 +60,28 @@ describe "#disallow chmod on some directory" do it "formula does a chmod to opt" do - expect { sandbox.exec "chmod", "ug-w", HOMEBREW_PREFIX}.to raise_error(ErrorDuringExecution) + expect { sandbox.exec "chmod", "ug-w", HOMEBREW_PREFIX }.to raise_error(ErrorDuringExecution) end it "allows chmod on a path allowed to write" do mktmpdir do |path| FileUtils.touch path/"foo" sandbox.allow_write_path(path) - expect { sandbox.exec "chmod", "ug-w", path/"foo"}.not_to raise_error(ErrorDuringExecution) + expect { sandbox.exec "chmod", "ug-w", path/"foo" }.not_to raise_error(ErrorDuringExecution) + end + end + end + + describe "#disallow chmod SUID or SGID on some directory" do + it "formula does a chmod 4000 to opt" do + expect { sandbox.exec "chmod", "4000", HOMEBREW_PREFIX }.to raise_error(ErrorDuringExecution) + end + + it "allows chmod 4000 on a path allowed to write" do + mktmpdir do |path| + FileUtils.touch path/"foo" + sandbox.allow_write_path(path) + expect { sandbox.exec "chmod", "4000", path/"foo" }.not_to raise_error(ErrorDuringExecution) end end end