CVE-2023-28630: Insertion of Sensitive Information into Log File (CWE-532)
value: GoCD is an open source continuous delivery server. In GoCD versions from 20.5.0 and below 23.1.0, if the server environment is not correctly configured by administrators to provide access to the relevant PostgreSQL or MySQL backup tools, the credentials for database access may be unintentionally leaked to admin alerts on the GoCD user interface. The vulnerability is triggered only if the GoCD server host is misconfigured to have backups enabled, but does not have access to the `pg_dump` or `mysqldump` utility tools to backup the configured database type (PostgreSQL or MySQL respectively). In such cases, failure to launch the expected backup utility reports the shell environment used to attempt to launch in the server admin alert, which includes the plaintext database password supplied to the configured tool. This vulnerability does not affect backups of the default on-disk H2 database that GoCD is configured to use. This issue has been addressed and fixed in GoCD 23.1.0. Users are advised to upgrade. Users unable to upgrade may disable backups, or administrators should ensure that the required `pg_dump` (PostgreSQL) or `mysqldump` (MySQL) binaries are available on the GoCD server when backups are triggered.
CWE description: Information written to log files can be of a sensitive nature and give valuable guidance to an attacker or expose sensitive user information.
CVSS v3 base score: 4.4
Published: 2023-03-27T21:15Z
Repository: https://github.com/gocd/gocd
Language: Java
Files changed: db-support/db-support-base/src/main/java/com/thoughtworks/go/server/database/BackupProcessor.java, db-support/db-support-mysql/src/main/java/com/thoughtworks/go/server/database/mysql/MySQLBackupProcessor.java, db-support/db-support-postgresql/src/main/java/com/thoughtworks/go/server/database/pg/PostgresqlBackupProcessor.java
Commit date: 2022-11-24 23:58:31 +0800
Commit message: Improve error messages on failure to launch backup process

ztexec can include env vars in the error message which we don't want in this case.

Diff: diff --git a/db-support/db-support-base/src/main/java/com/thoughtworks/go/server/database/BackupProcessor.java b/db-support/db-support-base/src/main/java/com/thoughtworks/go/server/database/BackupProcessor.java
index 4099cc4d04d..556ddaca398 100644
--- a/db-support/db-support-base/src/main/java/com/thoughtworks/go/server/database/BackupProcessor.java
+++ b/db-support/db-support-base/src/main/java/com/thoughtworks/go/server/database/BackupProcessor.java
@@ -21,6 +21,14 @@ import java.io.File;
 
 public interface BackupProcessor {
     void backup(File targetDir, DataSource dataSource, DbProperties dbProperties) throws Exception;
 
     boolean accepts(String url);
+
+    default void throwBackupError(String command, int errorCode) {
+        throwBackupError(command, errorCode, null);
+    }
+
+    default void throwBackupError(String command, int errorCode, Throwable cause) {
+        throw new RuntimeException(String.format("There was an error backing up the database using `%s`. The `%s` process failed with code %s. Please see the server logs for more detail.", command, command, errorCode), cause);
+    }
 }
diff --git a/db-support/db-support-mysql/src/main/java/com/thoughtworks/go/server/database/mysql/MySQLBackupProcessor.java b/db-support/db-support-mysql/src/main/java/com/thoughtworks/go/server/database/mysql/MySQLBackupProcessor.java
index d816bba11b7..8521946cc7b 100644
--- a/db-support/db-support-mysql/src/main/java/com/thoughtworks/go/server/database/mysql/MySQLBackupProcessor.java
+++ b/db-support/db-support-mysql/src/main/java/com/thoughtworks/go/server/database/mysql/MySQLBackupProcessor.java
@@ -21,36 +21,39 @@ import com.mysql.cj.conf.HostInfo;
 import com.thoughtworks.go.server.database.BackupProcessor;
 import com.thoughtworks.go.server.database.DbProperties;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.tools.ant.types.Commandline;
 import org.zeroturnaround.exec.ProcessExecutor;
+import org.zeroturnaround.exec.ProcessInitException;
 import org.zeroturnaround.exec.ProcessResult;
 import org.zeroturnaround.exec.stream.slf4j.Slf4jStream;
 
 import javax.sql.DataSource;
 import java.io.File;
 import java.io.IOException;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.LinkedHashMap;
+import java.util.*;
 import java.util.concurrent.TimeoutException;
 
 import static org.apache.commons.lang3.StringUtils.isNotBlank;
 
 @Slf4j
 public class MySQLBackupProcessor implements BackupProcessor {
 
+    private static final String COMMAND = "mysqldump";
+
     @Override
     public void backup(File targetDir, DataSource dataSource, DbProperties dbProperties) throws InterruptedException, TimeoutException, IOException {
-        ProcessResult processResult = createProcessExecutor(targetDir, dbProperties).execute();
-
-        if (processResult.getExitValue() == 0) {
-            log.info("MySQL backup finished successfully.");
-        } else {
-            log.warn("There was an error backing up the database using `mysqldump`. The `mysqldump` process exited with status code {}.", processResult.getExitValue());
-            throw new RuntimeException("There was an error backing up the database using `mysqldump`. The `mysqldump` process exited with status code " + processResult.getExitValue() +
-                    ". Please see the server logs for more errors");
+        try {
+            ProcessResult processResult = createProcessExecutor(targetDir, dbProperties).execute();
+
+            if (processResult.getExitValue() == 0) {
+                log.info("MySQL backup finished successfully.");
+            } else {
+                throwBackupError(COMMAND, processResult.getExitValue());
+            }
+        } catch (ProcessInitException e) {
+            throwBackupError(COMMAND, e.getErrorCode(), e.getCause());
         }
     }
 
     @Override
     public boolean accepts(String url) {
@@ -58,20 +61,19 @@ public class MySQLBackupProcessor implements BackupProcessor {
     }
 
     private ProcessExecutor createProcessExecutor(File targetDir, DbProperties dbProperties) {
         ConnectionUrl connectionUrlInstance = ConnectionUrl.getConnectionUrlInstance(dbProperties.url(), dbProperties.connectionProperties());
 
-        LinkedHashMap<String, String> env = new LinkedHashMap<>();
+        Map<String, String> env = new LinkedHashMap<>();
         if (isNotBlank(dbProperties.password())) {
             env.put("MYSQL_PWD", dbProperties.password());
         }
         // override with any user specified environment
         env.putAll(dbProperties.extraBackupEnv());
 
-        ArrayList<String> argv = new ArrayList<>();
-        argv.add("mysqldump");
-
+        List<String> argv = new ArrayList<>();
+        argv.add(COMMAND);
 
         String dbName = connectionUrlInstance.getDatabase();
         HostInfo mainHost = connectionUrlInstance.getMainHost();
 
         if (mainHost != null) {
@@ -85,11 +87,11 @@ public class MySQLBackupProcessor implements BackupProcessor {
         // append any user specified args for mysqldump
         if (isNotBlank(dbProperties.extraBackupCommandArgs())) {
             Collections.addAll(argv, Commandline.translateCommandline(dbProperties.extraBackupCommandArgs()));
         }
 
-        argv.add("--result-file=" + new File(targetDir, "db." + dbName).toString());
+        argv.add("--result-file=" + new File(targetDir, "db." + dbName));
         argv.add(connectionUrlInstance.getDatabase());
 
         ProcessExecutor processExecutor = new ProcessExecutor();
         processExecutor.redirectOutputAlsoTo(Slf4jStream.of(getClass()).asDebug());
         processExecutor.redirectErrorAlsoTo(Slf4jStream.of(getClass()).asDebug());
diff --git a/db-support/db-support-postgresql/src/main/java/com/thoughtworks/go/server/database/pg/PostgresqlBackupProcessor.java b/db-support/db-support-postgresql/src/main/java/com/thoughtworks/go/server/database/pg/PostgresqlBackupProcessor.java
index 5ae325d7b08..16fcf10fc27 100644
--- a/db-support/db-support-postgresql/src/main/java/com/thoughtworks/go/server/database/pg/PostgresqlBackupProcessor.java
+++ b/db-support/db-support-postgresql/src/main/java/com/thoughtworks/go/server/database/pg/PostgresqlBackupProcessor.java
@@ -20,36 +20,37 @@ import com.thoughtworks.go.server.database.BackupProcessor;
 import com.thoughtworks.go.server.database.DbProperties;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.tools.ant.types.Commandline;
 import org.postgresql.Driver;
 import org.zeroturnaround.exec.ProcessExecutor;
+import org.zeroturnaround.exec.ProcessInitException;
 import org.zeroturnaround.exec.ProcessResult;
 import org.zeroturnaround.exec.stream.slf4j.Slf4jStream;
 
 import javax.sql.DataSource;
 import java.io.File;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.LinkedHashMap;
-import java.util.Properties;
+import java.util.*;
 
 import static org.apache.commons.lang3.StringUtils.isNotBlank;
 
 @Slf4j
 public class PostgresqlBackupProcessor implements BackupProcessor {
 
+    private static final String COMMAND = "pg_dump";
+
     @Override
     public void backup(File targetDir, DataSource dataSource, DbProperties dbProperties) throws Exception {
-        ProcessResult processResult = createProcessExecutor(targetDir, dbProperties).execute();
-
-        if (processResult.getExitValue() == 0) {
-            log.info("PostgreSQL backup finished successfully.");
-        } else {
-            log.warn("There was an error backing up the database using `pg_dump`. The `pg_dump` process exited with status code {}.", processResult.getExitValue());
-            throw new RuntimeException("There was an error backing up the database using `pg_dump`. The `pg_dump` process exited with status code " + processResult.getExitValue() +
-                    ". Please see the server logs for more errors.");
+        try {
+            ProcessResult processResult = createProcessExecutor(targetDir, dbProperties).execute();
 
+            if (processResult.getExitValue() == 0) {
+                log.info("PostgreSQL backup finished successfully.");
+            } else {
+                throwBackupError(COMMAND, processResult.getExitValue());
+            }
+        } catch (ProcessInitException e) {
+            throwBackupError(COMMAND, e.getErrorCode(), e.getCause());
         }
     }
 
     @Override
     public boolean accepts(String url) {
@@ -58,21 +59,22 @@ public class PostgresqlBackupProcessor implements BackupProcessor {
 
     ProcessExecutor createProcessExecutor(File targetDir, DbProperties dbProperties) {
         Properties connectionProperties = dbProperties.connectionProperties();
         Properties pgProperties = Driver.parseURL(dbProperties.url(), connectionProperties);
 
-        ArrayList<String> argv = new ArrayList<>();
-        LinkedHashMap<String, String> env = new LinkedHashMap<>();
+        Map<String, String> env = new LinkedHashMap<>();
         if (isNotBlank(dbProperties.password())) {
             env.put("PGPASSWORD", dbProperties.password());
         }
 
         // override with any user specified environment
         env.putAll(dbProperties.extraBackupEnv());
 
+        List<String> argv = new ArrayList<>();
+        argv.add(COMMAND);
+
         String dbName = pgProperties.getProperty("PGDBNAME");
-        argv.add("pg_dump");
         argv.add("--host=" + pgProperties.getProperty("PGHOST"));
         argv.add("--port=" + pgProperties.getProperty("PGPORT"));
         argv.add("--dbname=" + dbName);
         if (isNotBlank(dbProperties.user())) {
             argv.add("--username=" + dbProperties.user());

Vulnerable code: import java.util.ArrayList;
import java.util.Collections;
import java.util.LinkedHashMap;
        ProcessResult processResult = createProcessExecutor(targetDir, dbProperties).execute();

        if (processResult.getExitValue() == 0) {
            log.info("MySQL backup finished successfully.");
        } else {
            log.warn("There was an error backing up the database using `mysqldump`. The `mysqldump` process exited with status code {}.", processResult.getExitValue());
            throw new RuntimeException("There was an error backing up the database using `mysqldump`. The `mysqldump` process exited with status code " + processResult.getExitValue() +
                    ". Please see the server logs for more errors");
        LinkedHashMap<String, String> env = new LinkedHashMap<>();
        ArrayList<String> argv = new ArrayList<>();
        argv.add("mysqldump");

        argv.add("--result-file=" + new File(targetDir, "db." + dbName).toString());
import java.util.ArrayList;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.Properties;
        ProcessResult processResult = createProcessExecutor(targetDir, dbProperties).execute();

        if (processResult.getExitValue() == 0) {
            log.info("PostgreSQL backup finished successfully.");
        } else {
            log.warn("There was an error backing up the database using `pg_dump`. The `pg_dump` process exited with status code {}.", processResult.getExitValue());
            throw new RuntimeException("There was an error backing up the database using `pg_dump`. The `pg_dump` process exited with status code " + processResult.getExitValue() +
                    ". Please see the server logs for more errors.");
        ArrayList<String> argv = new ArrayList<>();
        LinkedHashMap<String, String> env = new LinkedHashMap<>();
        argv.add("pg_dump");
Fixed code: 
    default void throwBackupError(String command, int errorCode) {
        throwBackupError(command, errorCode, null);
    }

    default void throwBackupError(String command, int errorCode, Throwable cause) {
        throw new RuntimeException(String.format("There was an error backing up the database using `%s`. The `%s` process failed with code %s. Please see the server logs for more detail.", command, command, errorCode), cause);
    }
import org.zeroturnaround.exec.ProcessInitException;
import java.util.*;
    private static final String COMMAND = "mysqldump";

        try {
            ProcessResult processResult = createProcessExecutor(targetDir, dbProperties).execute();

            if (processResult.getExitValue() == 0) {
                log.info("MySQL backup finished successfully.");
            } else {
                throwBackupError(COMMAND, processResult.getExitValue());
            }
        } catch (ProcessInitException e) {
            throwBackupError(COMMAND, e.getErrorCode(), e.getCause());
        Map<String, String> env = new LinkedHashMap<>();
        List<String> argv = new ArrayList<>();
        argv.add(COMMAND);
        argv.add("--result-file=" + new File(targetDir, "db." + dbName));
import org.zeroturnaround.exec.ProcessInitException;
import java.util.*;
    private static final String COMMAND = "pg_dump";

        try {
            ProcessResult processResult = createProcessExecutor(targetDir, dbProperties).execute();
            if (processResult.getExitValue() == 0) {
                log.info("PostgreSQL backup finished successfully.");
            } else {
                throwBackupError(COMMAND, processResult.getExitValue());
            }
        } catch (ProcessInitException e) {
            throwBackupError(COMMAND, e.getErrorCode(), e.getCause());
        Map<String, String> env = new LinkedHashMap<>();
        List<String> argv = new ArrayList<>();
        argv.add(COMMAND);