Skip to content

Commit 97d25a4

Browse files
authored
Merge pull request #69 from artemeon/fix/#22989-refactor-dbImport
Fix/#22989 refactor db import
2 parents 9ace64c + be80fb9 commit 97d25a4

10 files changed

+378
-930
lines changed

CHANGELOG.md

+5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
# Changelog
22

3+
## 3.3.1
4+
5+
- Fix Command Injection
6+
- Remove Oracle Driver
7+
38
## 3.3.0
49

510
- Remove unused SqlsrvDriver #68

composer.json

+4
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@
1919
"symfony/process": "^5.0|^6.0|^7.0",
2020
"symfony/polyfill-mbstring": "^1.15"
2121
},
22+
"suggest": {
23+
"ext-pgsql": "install for postgres.",
24+
"ext-mysqli": "install for mysql."
25+
},
2226
"require-dev": {
2327
"phpunit/phpunit": "^9.0",
2428
"phpstan/phpstan": "^1.10.59",

src/Driver/DriverAbstract.php

+12-4
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
namespace Artemeon\Database\Driver;
1515

1616
use Artemeon\Database\ConnectionInterface;
17+
use Artemeon\Database\ConnectionParameters;
1718
use Artemeon\Database\DriverInterface;
1819
use Artemeon\Database\Exception\QueryException;
1920
use Artemeon\Database\Schema\DataType;
@@ -32,6 +33,12 @@ abstract class DriverAbstract implements DriverInterface
3233

3334
protected int $affectedRowsCount = 0;
3435

36+
protected ?ConnectionParameters $config;
37+
38+
public function setConfig(ConnectionParameters $params): void
39+
{
40+
$this->config = $params;
41+
}
3542

3643
/**
3744
* Detects if the current installation runs on Windows or UNIX.
@@ -316,14 +323,15 @@ public function getStringLengthExpression(string $targetString): string
316323
return 'LENGTH('.$targetString.')';
317324
}
318325

319-
protected function runCommand(string $command): void
326+
protected function runProcess(Process $process, string $errorPrefix = ''): bool
320327
{
321-
$process = Process::fromShellCommandline($command);
322-
$process->setTimeout(3600.0);
328+
$process->setTimeout(null);
323329
$process->run();
324330

325331
if (!$process->isSuccessful()) {
326-
throw new ProcessFailedException($process);
332+
throw new \RuntimeException(trim($errorPrefix . ' ' . $process->getErrorOutput()));
327333
}
334+
335+
return true;
328336
}
329337
}

src/Driver/MysqliDriver.php

+44-30
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
use mysqli;
2626
use mysqli_stmt;
2727
use Symfony\Component\Process\ExecutableFinder;
28+
use Symfony\Component\Process\Process;
2829

2930
/**
3031
* DB-driver for MySQL using the php-mysqli-interface.
@@ -38,8 +39,6 @@ class MysqliDriver extends DriverAbstract
3839

3940
private ?mysqli $linkDB; //DB-Link
4041

41-
private ?ConnectionParameters $config;
42-
4342
private string $dumpBin = 'mysqldump'; // Binary to dump db (if not in path, add the path here)
4443

4544
private string $restoreBin = 'mysql'; // Binary to dump db (if not in path, add the path here)
@@ -62,7 +61,7 @@ public function dbconnect(ConnectionParameters $params): bool
6261
}
6362

6463
// Save connection-details
65-
$this->config = $params;
64+
$this->setConfig($params);
6665

6766
$this->linkDB = new mysqli(
6867
$this->config->getHost(),
@@ -500,27 +499,31 @@ public function encloseTableName(string $table): string
500499
*/
501500
public function dbExport(string &$fileName, array $tables): bool
502501
{
503-
$tablesString = implode(' ', $tables);
504-
$paramPass = '';
505-
506-
if ($this->config->getPassword() !== '') {
507-
$paramPass = " -p\"" . $this->config->getPassword() . "\"";
508-
}
509-
510502
$dumpBin = (new ExecutableFinder())->find($this->dumpBin);
503+
$dumpParams = [
504+
$dumpBin,
505+
'-h', escapeshellarg($this->config->getHost()),
506+
'-u', escapeshellarg($this->config->getUsername()),
507+
($this->config->getPassword() === '') ? '': '-p' . escapeshellarg($this->config->getPassword()),
508+
'-P', $this->config->getPort(),
509+
escapeshellarg($this->config->getDatabase()),
510+
implode(' ', array_map('escapeshellarg', $tables))
511+
];
511512

513+
$mysqldumpCommand = implode(' ', $dumpParams);
512514
if ($this->handlesDumpCompression()) {
513515
$fileName .= '.gz';
514-
$command = $dumpBin . ' -h' . $this->config->getHost() . ' -u' . $this->config->getUsername(
515-
) . $paramPass . ' -P' . $this->config->getPort() . ' ' . $this->config->getDatabase(
516-
) . ' ' . $tablesString . " | gzip > \"" . $fileName . "\"";
516+
$pattern = '%s | gzip > %s';
517517
} else {
518-
$command = $dumpBin . ' -h' . $this->config->getHost() . ' -u' . $this->config->getUsername(
519-
) . $paramPass . ' -P' . $this->config->getPort() . ' ' . $this->config->getDatabase(
520-
) . ' ' . $tablesString . " > \"" . $fileName . "\"";
518+
$pattern = '%s > %s';
521519
}
522520

523-
$this->runCommand($command);
521+
$process = new Process([
522+
'bash', '-c',
523+
sprintf($pattern, $mysqldumpCommand, escapeshellarg($fileName))
524+
]);
525+
526+
$this->runProcess($process, 'Database import failed:');
524527

525528
return true;
526529
}
@@ -530,33 +533,44 @@ public function dbExport(string &$fileName, array $tables): bool
530533
*/
531534
public function dbImport(string $fileName): bool
532535
{
533-
$paramPass = '';
534-
535-
if ($this->config->getPassword() !== '') {
536-
$paramPass = " -p\"" . $this->config->getPassword() . "\"";
536+
if (!in_array(pathinfo($fileName, PATHINFO_EXTENSION), ['sql', 'gz'])) {
537+
throw new \RuntimeException(trim($fileName . ' is not a valid import file'));
537538
}
538539

539540
$restoreBin = (new ExecutableFinder())->find($this->restoreBin);
540541

542+
$restoreParams = [
543+
$restoreBin,
544+
'-h', escapeshellarg($this->config->getHost()),
545+
'-u', escapeshellarg($this->config->getUsername()),
546+
($this->config->getPassword() === '') ? '': '-p' . escapeshellarg($this->config->getPassword()),
547+
'-P', $this->config->getPort(),
548+
escapeshellarg($this->config->getDatabase()),
549+
];
550+
551+
$mysqlCommand = implode(' ', $restoreParams);
541552
if ($this->handlesDumpCompression() && pathinfo($fileName, PATHINFO_EXTENSION) === 'gz') {
542-
$command = " gunzip -c \"" . $fileName . "\" | " . $restoreBin . ' -h' . $this->config->getHost(
543-
) . ' -u' . $this->config->getUsername() . $paramPass . ' -P' . $this->config->getPort(
544-
) . ' ' . $this->config->getDatabase();
553+
$fileCommand = sprintf('gunzip -c %s', escapeshellarg($fileName));
554+
} elseif (pathinfo($fileName, PATHINFO_EXTENSION) === 'sql') {
555+
$fileCommand = sprintf('cat %s', escapeshellarg($fileName));
545556
} else {
546-
$command = $restoreBin . ' -h' . $this->config->getHost() . ' -u' . $this->config->getUsername(
547-
) . $paramPass . ' -P' . $this->config->getPort() . ' ' . $this->config->getDatabase(
548-
) . " < \"" . $fileName . "\"";
557+
throw new \RuntimeException(trim($fileName . ' is not a valid import file'));
549558
}
550559

551-
$this->runCommand($command);
560+
$process = new Process([
561+
'bash', '-c',
562+
sprintf('%s | %s', $fileCommand, $mysqlCommand)
563+
]);
564+
565+
$this->runProcess($process, 'Database import failed:');
552566

553567
return true;
554568
}
555569

556570
/**
557571
* Prepares a statement or uses an instance from the cache.
558572
*/
559-
private function getPreparedStatement(string $query): mysqli_stmt | false
573+
private function getPreparedStatement(string $query): mysqli_stmt|false
560574
{
561575
$name = md5($query);
562576

@@ -587,7 +601,7 @@ private function getPreparedStatement(string $query): mysqli_stmt | false
587601

588602
public function escape(mixed $value): string
589603
{
590-
return str_replace("\\", "\\\\", (string) $value);
604+
return str_replace("\\", "\\\\", (string)$value);
591605
}
592606

593607
public function getJsonColumnExpression(string $column, string $key): string

0 commit comments

Comments
 (0)