CVE-2024-27294: Insufficient Information (NVD-CWE-noinfo)
value: dp-golang is a Puppet module for Go installations.  Prior to 1.2.7, dp-golang could install files — including the compiler binary — with the wrong ownership when Puppet was run as root and the installed package was On macOS: Go version 1.4.3 through 1.21rc3, inclusive, go1.4-bootstrap-20170518.tar.gz, or go1.4-bootstrap-20170531.tar.gz. The user and group specified in Puppet code were ignored for files within the archive. dp-puppet version 1.2.7 will recreate installations if the owner or group of any file or directory within that installation does not match the requested owner or group
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-02-29T23:15Z
Repository: https://github.com/danielparks/puppet-golang
Language: Other
Files changed: CHANGELOG.md, manifests/from_tarball.pp, spec/acceptance/golang_from_tarball_spec.rb
Commit date: 2024-02-29 12:18:31 -0800
Commit message: Enforce owner/group of Go installations

This now recreates a Go installation if any of its files or directories
have the wrong owner or group.

Diff: diff --git a/CHANGELOG.md b/CHANGELOG.md
index f913821..2832564 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -34,10 +34,17 @@ GitHub security advisory: [GHSA-8h8m-h98f-vv84]
   * go1.4-bootstrap-20170531.tar.gz
 
 [`tar`]: https://www.man7.org/linux/man-pages/man1/tar.1.html
 [GHSA-8h8m-h98f-vv84]: https://github.com/danielparks/puppet-golang/security/advisories/GHSA-8h8m-h98f-vv84
 
+### Changes
+
+As part of the security fix mentioned above, it became necessary to be more
+agressive about ensuring that the owner and group of files in the installation
+are correct. dp-golang now deletes and recreates any Go installation it finds
+that has a file or directory with the wrong owner or group.
+
 ## Release 1.2.6
 
 * Synced with [PDK][].
 
 [PDK]: https://www.puppet.com/docs/pdk/latest/pdk.html
diff --git a/manifests/from_tarball.pp b/manifests/from_tarball.pp
index 6c2521a..0f2c307 100644
--- a/manifests/from_tarball.pp
+++ b/manifests/from_tarball.pp
@@ -44,10 +44,13 @@ define golang::from_tarball (
   Variant[String[1], Integer[0]]     $owner      = $facts['identity']['user'],
   Variant[String[1], Integer[0]]     $group      = $facts['identity']['group'],
   String[1]                          $mode       = '0755',
   Stdlib::Unixpath                   $state_file = golang::state_file($go_dir),
 ) {
+  $encoded_go_dir = $go_dir.regsubst('/', '_', 'G')
+  $archive_path = "/tmp/puppet-golang${encoded_go_dir}.tar.gz"
+
   if $ensure != any_version {
     # Used to ensure that the installation is updated when $source changes.
     $file_ensure = $ensure ? {
       'present' => file,
       'absent'  => absent,
@@ -68,10 +71,42 @@ define golang::from_tarball (
         | EOF
       # lint:endignore
     }
   }
 
+  if $ensure == present or $ensure == any_version {
+    # Remove Go installation if any of its files have the wrong user or group.
+    # This will cause it to be replaced with a fresh installation.
+    exec { "dp/golang check ownership of ${go_dir}":
+      command     => ['rm', '-rf', $go_dir],
+      environment => [
+        "GO_DIR=${go_dir}",
+        "OWNER=${owner}",
+        "GROUP=${group}",
+      ],
+      path        => ['/usr/local/bin', '/usr/bin', '/bin'],
+      onlyif      => 'find "$GO_DIR" "(" "(" -not -user "$OWNER" ")" -or "(" -not -group "$GROUP" ")" ")" -print -quit | grep .',
+      before      => File[$go_dir],
+      notify      => Archive[$archive_path],
+    }
+  }
+
+  # File[$state_file] changing should only trigger an update when ensure is
+  # present, and not any_version.
+  if $ensure == present {
+    # If the $go_dir/bin directory exists, archive won't update it. Also, we
+    # want to remove any files that are not present in the new version.
+    exec { "dp/golang refresh go installation at ${go_dir}":
+      command     => ['rm', '-rf', $go_dir],
+      path        => ['/usr/local/bin', '/usr/bin', '/bin'],
+      refreshonly => true,
+      subscribe   => File[$state_file],
+      before      => File[$go_dir],
+      notify      => Archive[$archive_path],
+    }
+  }
+
   $directory_ensure = $ensure ? {
     'present'     => directory,
     'any_version' => directory,
     'absent'      => absent,
   }
@@ -83,28 +118,10 @@ define golang::from_tarball (
     group  => $group,
     mode   => $mode,
   }
 
   if $ensure == present or $ensure == any_version {
-    $encoded_go_dir = $go_dir.regsubst('/', '_', 'G')
-    $archive_path = "/tmp/puppet-golang${encoded_go_dir}.tar.gz"
-
-    # Only trigger an update when ensure is present, and not any_version.
-    if $ensure == present {
-      # If the $go_dir/bin directory exists, archive won't update it. Also, we
-      # want to remove any files that are not present in the new version.
-      exec { "dp/golang refresh go installation at ${go_dir}":
-        command     => ['rm', '-rf', $go_dir],
-        path        => ['/usr/local/bin', '/usr/bin', '/bin'],
-        user        => $facts['identity']['user'],
-        refreshonly => true,
-        subscribe   => File[$state_file],
-        before      => File[$go_dir],
-        notify      => Archive[$archive_path],
-      }
-    }
-
     include archive
 
     archive { $archive_path:
       ensure        => present,
       extract       => true,
diff --git a/spec/acceptance/golang_from_tarball_spec.rb b/spec/acceptance/golang_from_tarball_spec.rb
index 7e2dfa1..946a317 100644
--- a/spec/acceptance/golang_from_tarball_spec.rb
+++ b/spec/acceptance/golang_from_tarball_spec.rb
@@ -183,10 +183,52 @@ describe 'defined type golang::from_tarball' do
       describe file("#{home}/user/go-install/VERSION") do
         its(:content) { is_expected.to eq 'go1.10.4' }
         its(:owner) { is_expected.to eq 'user' }
         its(:group) { is_expected.to eq 'user' }
       end
+
+      context 'enforcing file ownership' do
+        it 'chowns bin/go' do
+          File.chown(0, 0, "#{home}/user/go-install/bin/go")
+        end
+
+        it 'reinstalls Go' do
+          apply_manifest(<<~"PUPPET", expect_changes: true)
+            golang::from_tarball { '#{home}/user/go-install':
+              source => '#{source_url}',
+              owner  => 'user',
+              group  => 'user',
+              mode   => '0700',
+            }
+          PUPPET
+        end
+
+        describe file("#{home}/user/go-install") do
+          it { is_expected.to be_directory }
+          its(:mode) { is_expected.to eq '700' }
+          its(:owner) { is_expected.to eq 'user' }
+          its(:group) { is_expected.to eq 'user' }
+        end
+
+        describe file("#{home}/user/go-install/bin/go") do
+          it { is_expected.to be_file }
+          its(:mode) { is_expected.to eq '755' }
+          its(:owner) { is_expected.to eq 'user' }
+          its(:group) { is_expected.to eq 'user' }
+        end
+
+        it 'does nothing' do
+          apply_manifest(<<~"PUPPET", catch_changes: true)
+            golang::from_tarball { '#{home}/user/go-install':
+              source => '#{source_url}',
+              owner  => 'user',
+              group  => 'user',
+              mode   => '0700',
+            }
+          PUPPET
+        end
+      end
     end
 
     context 'cleans up' do
       it 'uninstalls Go' do
         idempotent_apply(<<~"PUPPET")

Vulnerable code:     $encoded_go_dir = $go_dir.regsubst('/', '_', 'G')
    $archive_path = "/tmp/puppet-golang${encoded_go_dir}.tar.gz"

    # Only trigger an update when ensure is present, and not any_version.
    if $ensure == present {
      # If the $go_dir/bin directory exists, archive won't update it. Also, we
      # want to remove any files that are not present in the new version.
      exec { "dp/golang refresh go installation at ${go_dir}":
        command     => ['rm', '-rf', $go_dir],
        path        => ['/usr/local/bin', '/usr/bin', '/bin'],
        user        => $facts['identity']['user'],
        refreshonly => true,
        subscribe   => File[$state_file],
        before      => File[$go_dir],
        notify      => Archive[$archive_path],
      }
    }

Fixed code: ### Changes

As part of the security fix mentioned above, it became necessary to be more
agressive about ensuring that the owner and group of files in the installation
are correct. dp-golang now deletes and recreates any Go installation it finds
that has a file or directory with the wrong owner or group.

  $encoded_go_dir = $go_dir.regsubst('/', '_', 'G')
  $archive_path = "/tmp/puppet-golang${encoded_go_dir}.tar.gz"

  if $ensure == present or $ensure == any_version {
    # Remove Go installation if any of its files have the wrong user or group.
    # This will cause it to be replaced with a fresh installation.
    exec { "dp/golang check ownership of ${go_dir}":
      command     => ['rm', '-rf', $go_dir],
      environment => [
        "GO_DIR=${go_dir}",
        "OWNER=${owner}",
        "GROUP=${group}",
      ],
      path        => ['/usr/local/bin', '/usr/bin', '/bin'],
      onlyif      => 'find "$GO_DIR" "(" "(" -not -user "$OWNER" ")" -or "(" -not -group "$GROUP" ")" ")" -print -quit | grep .',
      before      => File[$go_dir],
      notify      => Archive[$archive_path],
    }
  }

  # File[$state_file] changing should only trigger an update when ensure is
  # present, and not any_version.
  if $ensure == present {
    # If the $go_dir/bin directory exists, archive won't update it. Also, we
    # want to remove any files that are not present in the new version.
    exec { "dp/golang refresh go installation at ${go_dir}":
      command     => ['rm', '-rf', $go_dir],
      path        => ['/usr/local/bin', '/usr/bin', '/bin'],
      refreshonly => true,
      subscribe   => File[$state_file],
      before      => File[$go_dir],
      notify      => Archive[$archive_path],
    }
  }


      context 'enforcing file ownership' do
        it 'chowns bin/go' do
          File.chown(0, 0, "#{home}/user/go-install/bin/go")
        end

        it 'reinstalls Go' do
          apply_manifest(<<~"PUPPET", expect_changes: true)
            golang::from_tarball { '#{home}/user/go-install':
              source => '#{source_url}',
              owner  => 'user',
              group  => 'user',
              mode   => '0700',
            }
          PUPPET
        end

        describe file("#{home}/user/go-install") do
          it { is_expected.to be_directory }
          its(:mode) { is_expected.to eq '700' }
          its(:owner) { is_expected.to eq 'user' }
          its(:group) { is_expected.to eq 'user' }
        end

        describe file("#{home}/user/go-install/bin/go") do
          it { is_expected.to be_file }
          its(:mode) { is_expected.to eq '755' }
          its(:owner) { is_expected.to eq 'user' }
          its(:group) { is_expected.to eq 'user' }
        end

        it 'does nothing' do
          apply_manifest(<<~"PUPPET", catch_changes: true)
            golang::from_tarball { '#{home}/user/go-install':
              source => '#{source_url}',
              owner  => 'user',
              group  => 'user',
              mode   => '0700',
            }
          PUPPET
        end
      end