From 7d136f997015b258f74fd15812b72fb9ee693757 Mon Sep 17 00:00:00 2001 From: Brady Wetherington Date: Thu, 14 Dec 2023 12:38:59 +0000 Subject: [PATCH 1/6] Initial rough stabs at prefix removal and SQL sanitization --- app/Console/Commands/RestoreFromBackup.php | 147 ++++++++++++++++++++- composer.json | 3 +- 2 files changed, 143 insertions(+), 7 deletions(-) diff --git a/app/Console/Commands/RestoreFromBackup.php b/app/Console/Commands/RestoreFromBackup.php index 7108568c5c..e7a8d8d57b 100644 --- a/app/Console/Commands/RestoreFromBackup.php +++ b/app/Console/Commands/RestoreFromBackup.php @@ -14,8 +14,10 @@ class RestoreFromBackup extends Command */ protected $signature = 'snipeit:restore {--force : Skip the danger prompt; assuming you enter "y"} - {filename : The zip file to be migrated} - {--no-progress : Don\'t show a progress bar}'; + {filename? : The zip file to be migrated} + {--no-progress : Don\'t show a progress bar} + {--sanitize-only : Sanitize and return SQL from STDIN} + {--prefix= : Don\'t guess DB table prefix; use the passed-in one (or none if just \'--prefix=\' is passed) }'; /** * The console command description. @@ -35,6 +37,113 @@ class RestoreFromBackup extends Command } public static $buffer_size = 1024 * 1024; // use a 1MB buffer, ought to work fine for most cases? + public static $prefix = null; + public static $tablenames = []; + + + public static function parse_sql(string $line,bool $should_guess = false): string + { + static $is_permitted = false; + // 'continuation' lines for a permitted statement are PERMITTED. + if($is_permitted && $line[0] === ' ') { + return $line; + } + + $table_regex = '`?([a-zA-Z0-9_]+)`?'; + $allowed_statements = [ + "/^(DROP TABLE (?:IF EXISTS )?)`$table_regex(.*)$/" => false, + "/^(CREATE TABLE )$table_regex(.*)$/" => true, //sets up 'continuation' + "/^(LOCK TABLES )$table_regex(.*)$/" => false, + "/^(INSERT INTO )$table_regex(.*)$/" => false, + "/^UNLOCK TABLES/" => false, + "/^\\) ENGINE=InnoDB AUTO_INCREMENT=16 DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;/" => false, // FIXME not sure what to do here? + // ^^^^^^ that bit should *exit* the 'perimitted' black + ]; + + foreach($allowed_statements as $statement => $statechange) { +// $this->info("Checking regex: $statement...\n"); + $matches = []; + if (preg_match($statement,$line,$matches)) { + $is_permitted = $statechange; + // matches are: 1 => first part of the statement, 2 => tablename, 3 => rest of statement + // (with of course 0 being "the whole match") + if (@$matches[2]) { + print "Found a tablename! It's: ".$matches[2]."\n"; + if ($should_guess) { + @self::$tablenames[$matches[2]] += 1; + continue; //oh? FIXME + } else { + $cleaned_tablename = \DB::getTablePrefix().preg_replace('/^'.self::$prefix,'',$matches[2]); + $line = preg_replace($statement,'$1`'.$cleaned_tablename.'`$3' , $line); + } + } else { + // no explicit tablename in this one, leave the line alone + } + //how do we *replace* the tablename? + print "RETURNING LINE: $line"; + return $line; + } + } + // all that is not allowed is denied. + return ""; + } + + public function guess_prefix($stream) //FIXME - oustream? I dunno + { + // try to determine prefix, I guess? + // should probably turn this into an instance method? Not sure. + // something where we can run this, pipe output to /dev/null? Dunno. + $bytes_read = 0; + $reading_beginning_of_line = true; + + while (($buffer = fgets($stream, self::$buffer_size)) !== false) { // FIXME this is copied, can we re-use? + $bytes_read += strlen($buffer); + if ($reading_beginning_of_line) { + $cleaned_buffer = self::parse_sql($buffer,true); //this isn't quite right? + fputs(STDOUT,$cleaned_buffer); + } + // if we got a newline at the end of this, then the _next_ read is the beginning of a line + if($buffer[strlen($buffer)-1] === "\n") { + $reading_beginning_of_line = true; + } else { + $reading_beginning_of_line = false; + } + + } + print_r(self::$tablenames); + + $check_tables = ['settings' => null, 'migrations' => null /* 'assets' => null */]; + //can't use 'users' because the 'accessories_users' table? + // can't use 'assets' because 'ver1_components_assets' + foreach($check_tables as $check_table => $_ignore) { + foreach (self::$tablenames as $tablename => $_count) { +// print "Comparing $tablename to $check_table\n"; + if (str_ends_with($tablename,$check_table)) { +// print "Found one!\n"; + $check_tables[$check_table] = substr($tablename,0,-strlen($check_table)); + } + } + } + $guessed_prefix = null; + foreach ($check_tables as $clean_table => $prefix_guess) { + if(is_null($prefix_guess)) { + print("Couldn't find table $clean_table\n"); + die(); + } + if(is_null($guessed_prefix)) { + $guessed_prefix = $prefix_guess; + } else { + if ($guessed_prefix != $prefix_guess) { + print("Prefix mismatch! Had guessed $guessed_prefix but got $prefix_guess\n"); + die(); + } + } + } + print "CALCULATED PREFIX: $guessed_prefix\n"; + self::$prefix = $guessed_prefix; + print_r($check_tables); + + } /** * Execute the console command. @@ -43,6 +152,18 @@ class RestoreFromBackup extends Command */ public function handle() { + if ( $this->option('prefix') ) { + self::$prefix = $this->option('prefix'); + } + + if ($this->option('sanitize-only')) { + if ( !self::$prefix) { + $this->guess_prefix(STDIN); + } + // for 'sanitize-only' - do we have to do something weird here, piping stuff around to stdin and stdout? + print "FINAL PREFIX IS: ".self::$prefix."\n"; + return true; + } $dir = getcwd(); if( $dir != base_path() ) { // usually only the case when running via webserver, not via command-line \Log::debug("Current working directory is: $dir, changing directory to: ".base_path()); @@ -240,7 +361,7 @@ class RestoreFromBackup extends Command $sql_stat = $za->statIndex($sqlfile_indices[0]); //$this->info("SQL Stat is: ".print_r($sql_stat,true)); - $sql_contents = $za->getStream($sql_stat['name']); + $sql_contents = $za->getStream($sql_stat['name']); /// duplicate this? if ($sql_contents === false) { $stdout = fgets($pipes[1]); $this->info($stdout); @@ -249,17 +370,31 @@ class RestoreFromBackup extends Command return false; } + $sql_prefix_sniff = $za->getStream($sql_stat['name']); /// duplicate this? + self::guess_prefix($sql_prefix_sniff); + $bytes_read = 0; + $reading_beginning_of_line = true; try { while (($buffer = fgets($sql_contents, self::$buffer_size)) !== false) { $bytes_read += strlen($buffer); - // \Log::debug("Buffer is: '$buffer'"); + if ($reading_beginning_of_line) { + //check allowlist? + // \Log::debug("Buffer is: '$buffer'"); $bytes_written = fwrite($pipes[0], $buffer); - if ($bytes_written === false) { - throw new Exception("Unable to write to pipe"); + if ($bytes_written === false) { + throw new Exception("Unable to write to pipe"); + } } + // if we got a newline at the end of this, then the _next_ read is the beginning of a line + if($buffer[strlen($buffer)-1] === "\n") { + $reading_beginning_of_line = true; + } else { + $reading_beginning_of_line = false; + } + } } catch (\Exception $e) { \Log::error("Error during restore!!!! ".$e->getMessage()); diff --git a/composer.json b/composer.json index 207d7d9f8c..0ff2cd999c 100644 --- a/composer.json +++ b/composer.json @@ -77,7 +77,8 @@ "watson/validating": "^6.1" }, "suggest": { - "ext-ldap": "*" + "ext-ldap": "*", + "ext-zip": "*" }, "require-dev": { "brianium/paratest": "^6.6", From 8c882ddead5a0f87055c4caf9c8a35982bd75ec2 Mon Sep 17 00:00:00 2001 From: Brady Wetherington Date: Thu, 14 Dec 2023 14:33:23 +0000 Subject: [PATCH 2/6] Starting to abstract out the SQL Streaming logic into its own class --- app/Console/Commands/RestoreFromBackup.php | 35 ++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/app/Console/Commands/RestoreFromBackup.php b/app/Console/Commands/RestoreFromBackup.php index e7a8d8d57b..7c2f003aae 100644 --- a/app/Console/Commands/RestoreFromBackup.php +++ b/app/Console/Commands/RestoreFromBackup.php @@ -5,6 +5,29 @@ namespace App\Console\Commands; use Illuminate\Console\Command; use ZipArchive; +class SQLStreamer { + private stream $input; + private ?stream $output; + // embed the prefix here? + + public function __construct(stream $input, ?stream $output) + { + // Iknow, I know, I'm getting there! Don't yell at me :( + $this->$input = $input; + $this->$output = $output; + } + + public function parse_sql() { + + } + + public static function guess_prefix() { + + } +} + + + class RestoreFromBackup extends Command { /** @@ -43,7 +66,7 @@ class RestoreFromBackup extends Command public static function parse_sql(string $line,bool $should_guess = false): string { - static $is_permitted = false; + static $is_permitted = false; // this 'static' is a code-smell. // 'continuation' lines for a permitted statement are PERMITTED. if($is_permitted && $line[0] === ' ') { return $line; @@ -94,7 +117,11 @@ class RestoreFromBackup extends Command // should probably turn this into an instance method? Not sure. // something where we can run this, pipe output to /dev/null? Dunno. $bytes_read = 0; - $reading_beginning_of_line = true; + $reading_beginning_of_line = true; //this is weird, because I kinda think it needs to 'leak' + // Like, if $reading_beginning_of_line is FALSE, we should be continuing whatever was happening before? + // e.g. you did a big giant chunk of INSERT statements - + // and it turned into *TWO* fgets's. That's fine, right? + // The *second* fgets should be treated how the *first* fgets was. while (($buffer = fgets($stream, self::$buffer_size)) !== false) { // FIXME this is copied, can we re-use? $bytes_read += strlen($buffer); @@ -163,6 +190,8 @@ class RestoreFromBackup extends Command // for 'sanitize-only' - do we have to do something weird here, piping stuff around to stdin and stdout? print "FINAL PREFIX IS: ".self::$prefix."\n"; return true; + // so this, to me, feels like *one* mode of the whatever-streamer we have here. + // we run *this* one with the guess first, then the 'real' one, and hand STDOUT over. } $dir = getcwd(); if( $dir != base_path() ) { // usually only the case when running via webserver, not via command-line @@ -376,6 +405,8 @@ class RestoreFromBackup extends Command $bytes_read = 0; $reading_beginning_of_line = true; + // so this whole bit seems like a dupe - but it's similar as the one that guesses tyings. + // The difference is our out-stream is now the pipe to mysql try { while (($buffer = fgets($sql_contents, self::$buffer_size)) !== false) { $bytes_read += strlen($buffer); From fcf023e3d2ca749c35ac6926514ff117b7a89c97 Mon Sep 17 00:00:00 2001 From: Brady Wetherington Date: Mon, 18 Dec 2023 17:03:26 +0000 Subject: [PATCH 3/6] WIP: trying to get prefixing and sanitization working --- app/Console/Commands/RestoreFromBackup.php | 25 +++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/app/Console/Commands/RestoreFromBackup.php b/app/Console/Commands/RestoreFromBackup.php index 7c2f003aae..c36d1a27d2 100644 --- a/app/Console/Commands/RestoreFromBackup.php +++ b/app/Console/Commands/RestoreFromBackup.php @@ -9,20 +9,34 @@ class SQLStreamer { private stream $input; private ?stream $output; // embed the prefix here? + public string $prefix; //maybe make it reach-able-into-able? + // another thing to note - 'wrapping' this prefix value into the object doesn't really help us - as STDIN and STDOUT + // (and ZIP streams) can't be rewound, I don't think (well, ZIP ones can't, for sure) + // - public function __construct(stream $input, ?stream $output) + private bool $at_start_of_line = true; + + public function __construct(stream $input, ?stream $output, string $prefix = null) // should we have a prefix here in the constructor? { // Iknow, I know, I'm getting there! Don't yell at me :( $this->$input = $input; $this->$output = $output; + $this->$prefix = $prefix; } - public function parse_sql() { - + public function parse_sql(string $line): string { + // take into account the 'start of line or not' setting as an instance variable? + return ""; } - public static function guess_prefix() { - + //this is used in exactly *TWO* places, and in both cases should return a prefix I think? + // first - if you do the --sanitize-only one (which is mostly for testing/development) + // next - when you run *without* a guessed prefix, this is run first to figure out the prefix + // I think we have to *duplicate* the call to be able to run it again? + public static function guess_prefix(stream $input):string { + // does *this* go and change the instance settings to assign a prefix? + // spin up a new instance of my own class, and run the parse_sql thing with the guess option? + // (or maybe set the guess option directly?) } } @@ -378,6 +392,7 @@ class RestoreFromBackup extends Command return $this->error('Unable to invoke mysql via CLI'); } + // I'm not sure about these? stream_set_blocking($pipes[1], false); // use non-blocking reads for stdout stream_set_blocking($pipes[2], false); // use non-blocking reads for stderr From 70ef904951bed064f0e89839cf862b51f216f12f Mon Sep 17 00:00:00 2001 From: Brady Wetherington Date: Tue, 19 Dec 2023 15:31:59 +0000 Subject: [PATCH 4/6] Actually got this pretty close to being able to do a restore --- app/Console/Commands/RestoreFromBackup.php | 282 +++++++++++---------- 1 file changed, 144 insertions(+), 138 deletions(-) diff --git a/app/Console/Commands/RestoreFromBackup.php b/app/Console/Commands/RestoreFromBackup.php index c36d1a27d2..21606f1afb 100644 --- a/app/Console/Commands/RestoreFromBackup.php +++ b/app/Console/Commands/RestoreFromBackup.php @@ -6,83 +6,30 @@ use Illuminate\Console\Command; use ZipArchive; class SQLStreamer { - private stream $input; - private ?stream $output; + private $input; + private $output; // embed the prefix here? - public string $prefix; //maybe make it reach-able-into-able? - // another thing to note - 'wrapping' this prefix value into the object doesn't really help us - as STDIN and STDOUT - // (and ZIP streams) can't be rewound, I don't think (well, ZIP ones can't, for sure) - // + public ?string $prefix; - private bool $at_start_of_line = true; + private bool $reading_beginning_of_line = true; - public function __construct(stream $input, ?stream $output, string $prefix = null) // should we have a prefix here in the constructor? + public static $buffer_size = 1024 * 1024; // use a 1MB buffer, ought to work fine for most cases? + + public array $tablenames = []; + private bool $should_guess = false; + private bool $statement_is_permitted = false; + + public function __construct($input, $output, string $prefix = null) { - // Iknow, I know, I'm getting there! Don't yell at me :( - $this->$input = $input; - $this->$output = $output; - $this->$prefix = $prefix; + $this->input = $input; + $this->output = $output; + $this->prefix = $prefix; } public function parse_sql(string $line): string { // take into account the 'start of line or not' setting as an instance variable? - return ""; - } - - //this is used in exactly *TWO* places, and in both cases should return a prefix I think? - // first - if you do the --sanitize-only one (which is mostly for testing/development) - // next - when you run *without* a guessed prefix, this is run first to figure out the prefix - // I think we have to *duplicate* the call to be able to run it again? - public static function guess_prefix(stream $input):string { - // does *this* go and change the instance settings to assign a prefix? - // spin up a new instance of my own class, and run the parse_sql thing with the guess option? - // (or maybe set the guess option directly?) - } -} - - - -class RestoreFromBackup extends Command -{ - /** - * The name and signature of the console command. - * - * @var string - */ - protected $signature = 'snipeit:restore - {--force : Skip the danger prompt; assuming you enter "y"} - {filename? : The zip file to be migrated} - {--no-progress : Don\'t show a progress bar} - {--sanitize-only : Sanitize and return SQL from STDIN} - {--prefix= : Don\'t guess DB table prefix; use the passed-in one (or none if just \'--prefix=\' is passed) }'; - - /** - * The console command description. - * - * @var string - */ - protected $description = 'Restore from a previously created Snipe-IT backup file'; - - /** - * Create a new command instance. - * - * @return void - */ - public function __construct() - { - parent::__construct(); - } - - public static $buffer_size = 1024 * 1024; // use a 1MB buffer, ought to work fine for most cases? - public static $prefix = null; - public static $tablenames = []; - - - public static function parse_sql(string $line,bool $should_guess = false): string - { - static $is_permitted = false; // this 'static' is a code-smell. // 'continuation' lines for a permitted statement are PERMITTED. - if($is_permitted && $line[0] === ' ') { + if($this->statement_is_permitted && $line[0] === ' ') { return $line; } @@ -93,7 +40,8 @@ class RestoreFromBackup extends Command "/^(LOCK TABLES )$table_regex(.*)$/" => false, "/^(INSERT INTO )$table_regex(.*)$/" => false, "/^UNLOCK TABLES/" => false, - "/^\\) ENGINE=InnoDB AUTO_INCREMENT=16 DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;/" => false, // FIXME not sure what to do here? + // "/^\\) ENGINE=InnoDB AUTO_INCREMENT=16 DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;/" => false, // FIXME not sure what to do here? + "/^\\)[a-zA-Z0-9_= ]*;$/" => false // ^^^^^^ that bit should *exit* the 'perimitted' black ]; @@ -101,23 +49,23 @@ class RestoreFromBackup extends Command // $this->info("Checking regex: $statement...\n"); $matches = []; if (preg_match($statement,$line,$matches)) { - $is_permitted = $statechange; + $this->statement_is_permitted = $statechange; // matches are: 1 => first part of the statement, 2 => tablename, 3 => rest of statement // (with of course 0 being "the whole match") if (@$matches[2]) { - print "Found a tablename! It's: ".$matches[2]."\n"; - if ($should_guess) { - @self::$tablenames[$matches[2]] += 1; +// print "Found a tablename! It's: ".$matches[2]."\n"; + if ($this->should_guess) { + @$this->tablenames[$matches[2]] += 1; continue; //oh? FIXME } else { - $cleaned_tablename = \DB::getTablePrefix().preg_replace('/^'.self::$prefix,'',$matches[2]); + $cleaned_tablename = \DB::getTablePrefix().preg_replace('/^'.$this->prefix.'/','',$matches[2]); $line = preg_replace($statement,'$1`'.$cleaned_tablename.'`$3' , $line); } } else { // no explicit tablename in this one, leave the line alone } //how do we *replace* the tablename? - print "RETURNING LINE: $line"; +// print "RETURNING LINE: $line"; return $line; } } @@ -125,39 +73,22 @@ class RestoreFromBackup extends Command return ""; } - public function guess_prefix($stream) //FIXME - oustream? I dunno + //this is used in exactly *TWO* places, and in both cases should return a prefix I think? + // first - if you do the --sanitize-only one (which is mostly for testing/development) + // next - when you run *without* a guessed prefix, this is run first to figure out the prefix + // I think we have to *duplicate* the call to be able to run it again? + public static function guess_prefix($input):string { - // try to determine prefix, I guess? - // should probably turn this into an instance method? Not sure. - // something where we can run this, pipe output to /dev/null? Dunno. - $bytes_read = 0; - $reading_beginning_of_line = true; //this is weird, because I kinda think it needs to 'leak' - // Like, if $reading_beginning_of_line is FALSE, we should be continuing whatever was happening before? - // e.g. you did a big giant chunk of INSERT statements - - // and it turned into *TWO* fgets's. That's fine, right? - // The *second* fgets should be treated how the *first* fgets was. + $parser = new self($input, null); + $parser->should_guess = true; + $parser->line_aware_piping(); // <----- THIS is doing the heavy lifting! + print_r($parser->tablenames); - while (($buffer = fgets($stream, self::$buffer_size)) !== false) { // FIXME this is copied, can we re-use? - $bytes_read += strlen($buffer); - if ($reading_beginning_of_line) { - $cleaned_buffer = self::parse_sql($buffer,true); //this isn't quite right? - fputs(STDOUT,$cleaned_buffer); - } - // if we got a newline at the end of this, then the _next_ read is the beginning of a line - if($buffer[strlen($buffer)-1] === "\n") { - $reading_beginning_of_line = true; - } else { - $reading_beginning_of_line = false; - } - - } - print_r(self::$tablenames); - - $check_tables = ['settings' => null, 'migrations' => null /* 'assets' => null */]; + $check_tables = ['settings' => null, 'migrations' => null /* 'assets' => null */]; //TODO - move to statics? //can't use 'users' because the 'accessories_users' table? // can't use 'assets' because 'ver1_components_assets' foreach($check_tables as $check_table => $_ignore) { - foreach (self::$tablenames as $tablename => $_count) { + foreach ($parser->tablenames as $tablename => $_count) { // print "Comparing $tablename to $check_table\n"; if (str_ends_with($tablename,$check_table)) { // print "Found one!\n"; @@ -181,11 +112,101 @@ class RestoreFromBackup extends Command } } print "CALCULATED PREFIX: $guessed_prefix\n"; - self::$prefix = $guessed_prefix; +// self::$prefix = $guessed_prefix; print_r($check_tables); + return $guessed_prefix; + } + public function line_aware_piping(): int + { + $bytes_read = 0; + if (! $this->input) { + print "Your input is all fucked up yo.\n"; + die("you suck"); + } + // FIXME - fix the indentation +// try { + while (($buffer = fgets($this->input, self::$buffer_size)) !== false) { + $bytes_read += strlen($buffer); + if ($this->reading_beginning_of_line) { + // \Log::debug("Buffer is: '$buffer'"); + $cleaned_buffer = $this->parse_sql($buffer); +// print "CLEANED BUFFER IS: $cleaned_buffer\n"; + if ($this->output) { + $bytes_written = fwrite($this->output, $cleaned_buffer); + + if ($bytes_written === false) { + throw new \Exception("Unable to write to pipe"); + } + } + } + // if we got a newline at the end of this, then the _next_ read is the beginning of a line + if($buffer[strlen($buffer)-1] === "\n") { + $this->reading_beginning_of_line = true; + } else { + $this->reading_beginning_of_line = false; + } + + } + return $bytes_read; +// } catch (\Exception $e) { //FIXME - move this out? Dunno. +// print "Uhm, what?\n"; +// +// dd($e); +// print_r($e); +// die(-1); +// \Log::error("Error during restore!!!! ".$e->getMessage()); +// // FIXME - put these back and/or put them in the right places?! +//// $err_out = fgets($pipes[1]); +//// $err_err = fgets($pipes[2]); +//// \Log::error("Error OUTPUT: ".$err_out); +//// $this->info($err_out); +//// \Log::error("Error ERROR : ".$err_err); +//// $this->error($err_err); +//// throw $e; +// } + + } +} + + + +class RestoreFromBackup extends Command +{ + /** + * The name and signature of the console command. + * + * @var string + */ + protected $signature = 'snipeit:restore + {--force : Skip the danger prompt; assuming you enter "y"} + {filename? : The zip file to be migrated} + {--no-progress : Don\'t show a progress bar} + {--sanitize-only : Sanitize and return SQL from STDIN} + {--prefix= : Don\'t guess DB table prefix; use the passed-in one (or none if just \'--prefix=\' is passed) } + {--no-sanitize : Don\'t try to sanitize the SQL at all; pass it through unmodified}'; + + /** + * The console command description. + * + * @var string + */ + protected $description = 'Restore from a previously created Snipe-IT backup file'; + + /** + * Create a new command instance. + * + * @return void + */ + public function __construct() + { + parent::__construct(); + } + + public static $prefix = null; + /** * Execute the console command. * @@ -199,13 +220,17 @@ class RestoreFromBackup extends Command if ($this->option('sanitize-only')) { if ( !self::$prefix) { - $this->guess_prefix(STDIN); + print "okay, no prefix declared, we're going to GUESS IT!!!!\n"; + self::$prefix = SQLStreamer::guess_prefix(STDIN); + print "FINAL PREFIX IS: ".self::$prefix."\n"; + return $this->info("Re-run this command with '--prefix=".self::$prefix."' to see an attempt to sanitze your SQL."); + } else { + // for 'sanitize-only' - do we have to do something weird here, piping stuff around to stdin and stdout? + $this->comment("OKIE DOKE!! Here we go to try and sanitize some bidness"); + $stream = new SQLStreamer(STDIN, STDOUT, self::$prefix); + $stream->line_aware_piping(); + return $this->info("WE ARE ALL DONE! YAY!!!!!!!!!"); } - // for 'sanitize-only' - do we have to do something weird here, piping stuff around to stdin and stdout? - print "FINAL PREFIX IS: ".self::$prefix."\n"; - return true; - // so this, to me, feels like *one* mode of the whatever-streamer we have here. - // we run *this* one with the guess first, then the 'real' one, and hand STDOUT over. } $dir = getcwd(); if( $dir != base_path() ) { // usually only the case when running via webserver, not via command-line @@ -405,7 +430,7 @@ class RestoreFromBackup extends Command $sql_stat = $za->statIndex($sqlfile_indices[0]); //$this->info("SQL Stat is: ".print_r($sql_stat,true)); - $sql_contents = $za->getStream($sql_stat['name']); /// duplicate this? + $sql_contents = $za->getStream($sql_stat['name']); if ($sql_contents === false) { $stdout = fgets($pipes[1]); $this->info($stdout); @@ -414,36 +439,18 @@ class RestoreFromBackup extends Command return false; } - $sql_prefix_sniff = $za->getStream($sql_stat['name']); /// duplicate this? - self::guess_prefix($sql_prefix_sniff); + if (! self::$prefix ) { + $sql_prefix_sniffer = $za->getStream($sql_stat['name']); + self::$prefix = SQLStreamer::guess_prefix($sql_prefix_sniffer); + $this->info("Guessed prefix: '".self::$prefix."'"); + } - $bytes_read = 0; - $reading_beginning_of_line = true; - - // so this whole bit seems like a dupe - but it's similar as the one that guesses tyings. - // The difference is our out-stream is now the pipe to mysql try { - while (($buffer = fgets($sql_contents, self::$buffer_size)) !== false) { - $bytes_read += strlen($buffer); - if ($reading_beginning_of_line) { - //check allowlist? - // \Log::debug("Buffer is: '$buffer'"); - $bytes_written = fwrite($pipes[0], $buffer); - - if ($bytes_written === false) { - throw new Exception("Unable to write to pipe"); - } - } - // if we got a newline at the end of this, then the _next_ read is the beginning of a line - if($buffer[strlen($buffer)-1] === "\n") { - $reading_beginning_of_line = true; - } else { - $reading_beginning_of_line = false; - } - - } + $sql_importer = new SQLStreamer($sql_contents, $pipes[0], self::$prefix); + $bytes_read = $sql_importer->line_aware_piping(); } catch (\Exception $e) { \Log::error("Error during restore!!!! ".$e->getMessage()); + // FIXME - put these back and/or put them in the right places?! $err_out = fgets($pipes[1]); $err_err = fgets($pipes[2]); \Log::error("Error OUTPUT: ".$err_out); @@ -452,7 +459,6 @@ class RestoreFromBackup extends Command $this->error($err_err); throw $e; } - if (!feof($sql_contents) || $bytes_read == 0) { return $this->error("Not at end of file for sql file, or zero bytes read. aborting!"); } From 955f75f7330942cc3efa02d3f2fa38642a55e5fc Mon Sep 17 00:00:00 2001 From: Brady Wetherington Date: Tue, 13 Feb 2024 16:45:24 +0000 Subject: [PATCH 5/6] Fixes for the prefix-guessing and sanitizing. --- app/Console/Commands/RestoreFromBackup.php | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/app/Console/Commands/RestoreFromBackup.php b/app/Console/Commands/RestoreFromBackup.php index 21606f1afb..d85e5fe921 100644 --- a/app/Console/Commands/RestoreFromBackup.php +++ b/app/Console/Commands/RestoreFromBackup.php @@ -214,12 +214,12 @@ class RestoreFromBackup extends Command */ public function handle() { - if ( $this->option('prefix') ) { + if ( $this->option('prefix') !== null ) { self::$prefix = $this->option('prefix'); } if ($this->option('sanitize-only')) { - if ( !self::$prefix) { + if ( self::$prefix === null) { print "okay, no prefix declared, we're going to GUESS IT!!!!\n"; self::$prefix = SQLStreamer::guess_prefix(STDIN); print "FINAL PREFIX IS: ".self::$prefix."\n"; @@ -446,8 +446,20 @@ class RestoreFromBackup extends Command } try { - $sql_importer = new SQLStreamer($sql_contents, $pipes[0], self::$prefix); - $bytes_read = $sql_importer->line_aware_piping(); + if ( $this->option('no-sanitize')) { + while (($buffer = fgets($sql_contents, self::$buffer_size)) !== false) { + $bytes_read += strlen($buffer); + // \Log::debug("Buffer is: '$buffer'"); + $bytes_written = fwrite($pipes[0], $buffer); + + if ($bytes_written === false) { + throw new Exception("Unable to write to pipe"); + } + } + } else { + $sql_importer = new SQLStreamer($sql_contents, $pipes[0], self::$prefix); + $bytes_read = $sql_importer->line_aware_piping(); + } } catch (\Exception $e) { \Log::error("Error during restore!!!! ".$e->getMessage()); // FIXME - put these back and/or put them in the right places?! From 6a78706a3e04467957de86a57d822a359e06acc5 Mon Sep 17 00:00:00 2001 From: Brady Wetherington Date: Thu, 15 Feb 2024 13:27:18 +0000 Subject: [PATCH 6/6] New 'sanitize' version of backup-restore tool. Optional. --- app/Console/Commands/RestoreFromBackup.php | 136 ++++++++------------- 1 file changed, 50 insertions(+), 86 deletions(-) diff --git a/app/Console/Commands/RestoreFromBackup.php b/app/Console/Commands/RestoreFromBackup.php index d85e5fe921..2133b22be1 100644 --- a/app/Console/Commands/RestoreFromBackup.php +++ b/app/Console/Commands/RestoreFromBackup.php @@ -82,7 +82,6 @@ class SQLStreamer { $parser = new self($input, null); $parser->should_guess = true; $parser->line_aware_piping(); // <----- THIS is doing the heavy lifting! - print_r($parser->tablenames); $check_tables = ['settings' => null, 'migrations' => null /* 'assets' => null */]; //TODO - move to statics? //can't use 'users' because the 'accessories_users' table? @@ -111,9 +110,6 @@ class SQLStreamer { } } } - print "CALCULATED PREFIX: $guessed_prefix\n"; -// self::$prefix = $guessed_prefix; - print_r($check_tables); return $guessed_prefix; @@ -123,50 +119,31 @@ class SQLStreamer { { $bytes_read = 0; if (! $this->input) { - print "Your input is all fucked up yo.\n"; - die("you suck"); + throw new \Exception("No Input available for line_aware_piping"); } - // FIXME - fix the indentation -// try { - while (($buffer = fgets($this->input, self::$buffer_size)) !== false) { - $bytes_read += strlen($buffer); - if ($this->reading_beginning_of_line) { - // \Log::debug("Buffer is: '$buffer'"); - $cleaned_buffer = $this->parse_sql($buffer); -// print "CLEANED BUFFER IS: $cleaned_buffer\n"; - if ($this->output) { - $bytes_written = fwrite($this->output, $cleaned_buffer); - if ($bytes_written === false) { - throw new \Exception("Unable to write to pipe"); - } + while (($buffer = fgets($this->input, SQLStreamer::$buffer_size)) !== false) { + $bytes_read += strlen($buffer); + if ($this->reading_beginning_of_line) { + // \Log::debug("Buffer is: '$buffer'"); + $cleaned_buffer = $this->parse_sql($buffer); + if ($this->output) { + $bytes_written = fwrite($this->output, $cleaned_buffer); + + if ($bytes_written === false) { + throw new \Exception("Unable to write to pipe"); } } - // if we got a newline at the end of this, then the _next_ read is the beginning of a line - if($buffer[strlen($buffer)-1] === "\n") { - $this->reading_beginning_of_line = true; - } else { - $this->reading_beginning_of_line = false; - } - } - return $bytes_read; -// } catch (\Exception $e) { //FIXME - move this out? Dunno. -// print "Uhm, what?\n"; -// -// dd($e); -// print_r($e); -// die(-1); -// \Log::error("Error during restore!!!! ".$e->getMessage()); -// // FIXME - put these back and/or put them in the right places?! -//// $err_out = fgets($pipes[1]); -//// $err_err = fgets($pipes[2]); -//// \Log::error("Error OUTPUT: ".$err_out); -//// $this->info($err_out); -//// \Log::error("Error ERROR : ".$err_err); -//// $this->error($err_err); -//// throw $e; -// } + // if we got a newline at the end of this, then the _next_ read is the beginning of a line + if($buffer[strlen($buffer)-1] === "\n") { + $this->reading_beginning_of_line = true; + } else { + $this->reading_beginning_of_line = false; + } + + } + return $bytes_read; } } @@ -180,13 +157,13 @@ class RestoreFromBackup extends Command * * @var string */ + // FIXME - , stripping prefixes and nonstandard SQL statements. Without --prefix, guess and return the correct prefix to strip protected $signature = 'snipeit:restore {--force : Skip the danger prompt; assuming you enter "y"} - {filename? : The zip file to be migrated} + {filename : The zip file to be migrated} {--no-progress : Don\'t show a progress bar} - {--sanitize-only : Sanitize and return SQL from STDIN} - {--prefix= : Don\'t guess DB table prefix; use the passed-in one (or none if just \'--prefix=\' is passed) } - {--no-sanitize : Don\'t try to sanitize the SQL at all; pass it through unmodified}'; + {--sanitize-guess-prefix : Guess and output the table-prefix needed to "sanitize" the SQL} + {--sanitize-with-prefix= : "Sanitize" the SQL, using the passed-in table prefix (can be learned from --sanitize-guess-prefix). Pass as just \'--sanitize-with-prefix=\' to use no prefix}'; /** * The console command description. @@ -205,8 +182,6 @@ class RestoreFromBackup extends Command parent::__construct(); } - public static $prefix = null; - /** * Execute the console command. * @@ -214,24 +189,6 @@ class RestoreFromBackup extends Command */ public function handle() { - if ( $this->option('prefix') !== null ) { - self::$prefix = $this->option('prefix'); - } - - if ($this->option('sanitize-only')) { - if ( self::$prefix === null) { - print "okay, no prefix declared, we're going to GUESS IT!!!!\n"; - self::$prefix = SQLStreamer::guess_prefix(STDIN); - print "FINAL PREFIX IS: ".self::$prefix."\n"; - return $this->info("Re-run this command with '--prefix=".self::$prefix."' to see an attempt to sanitze your SQL."); - } else { - // for 'sanitize-only' - do we have to do something weird here, piping stuff around to stdin and stdout? - $this->comment("OKIE DOKE!! Here we go to try and sanitize some bidness"); - $stream = new SQLStreamer(STDIN, STDOUT, self::$prefix); - $stream->line_aware_piping(); - return $this->info("WE ARE ALL DONE! YAY!!!!!!!!!"); - } - } $dir = getcwd(); if( $dir != base_path() ) { // usually only the case when running via webserver, not via command-line \Log::debug("Current working directory is: $dir, changing directory to: ".base_path()); @@ -244,7 +201,7 @@ class RestoreFromBackup extends Command return $this->error('Missing required filename'); } - if (! $this->option('force') && ! $this->confirm('Are you sure you wish to restore from the given backup file? This can lead to MASSIVE DATA LOSS!')) { + if (! $this->option('force') && ! $this->option('sanitize-guess-prefix') && ! $this->confirm('Are you sure you wish to restore from the given backup file? This can lead to MASSIVE DATA LOSS!')) { return $this->error('Data loss not confirmed'); } @@ -347,11 +304,11 @@ class RestoreFromBackup extends Command } foreach (array_merge($private_dirs, $public_dirs) as $dir) { - $last_pos = strrpos($raw_path, $dir.'/'); + $last_pos = strrpos($raw_path, $dir . '/'); if ($last_pos !== false) { //print("INTERESTING - last_pos is $last_pos when searching $raw_path for $dir - last_pos+strlen(\$dir) is: ".($last_pos+strlen($dir))." and strlen(\$rawpath) is: ".strlen($raw_path)."\n"); //print("We would copy $raw_path to $dir.\n"); //FIXME append to a path? - $interesting_files[$raw_path] = ['dest' =>$dir, 'index' => $i]; + $interesting_files[$raw_path] = ['dest' => $dir, 'index' => $i]; continue 2; if ($last_pos + strlen($dir) + 1 == strlen($raw_path)) { // we don't care about that; we just want files with the appropriate prefix @@ -360,7 +317,7 @@ class RestoreFromBackup extends Command } } $good_extensions = ['png', 'gif', 'jpg', 'svg', 'jpeg', 'doc', 'docx', 'pdf', 'txt', - 'zip', 'rar', 'xls', 'xlsx', 'lic', 'xml', 'rtf', 'webp', 'key', 'ico', ]; + 'zip', 'rar', 'xls', 'xlsx', 'lic', 'xml', 'rtf', 'webp', 'key', 'ico',]; foreach (array_merge($private_files, $public_files) as $file) { $has_wildcard = (strpos($file, '*') !== false); if ($has_wildcard) { @@ -369,8 +326,8 @@ class RestoreFromBackup extends Command $last_pos = strrpos($raw_path, $file); // no trailing slash! if ($last_pos !== false) { $extension = strtolower(pathinfo($raw_path, PATHINFO_EXTENSION)); - if (! in_array($extension, $good_extensions)) { - $this->warn('Potentially unsafe file '.$raw_path.' is being skipped'); + if (!in_array($extension, $good_extensions)) { + $this->warn('Potentially unsafe file ' . $raw_path . ' is being skipped'); $boring_files[] = $raw_path; continue 2; } @@ -385,7 +342,6 @@ class RestoreFromBackup extends Command } $boring_files[] = $raw_path; //if we've gotten to here and haven't continue'ed our way into the next iteration, we don't want this file } // end of pre-processing the ZIP file for-loop - // print_r($interesting_files);exit(-1); if (count($sqlfiles) != 1) { @@ -397,6 +353,17 @@ class RestoreFromBackup extends Command //older Snipe-IT installs don't have the db-dumps subdirectory component } + $sql_stat = $za->statIndex($sqlfile_indices[0]); + //$this->info("SQL Stat is: ".print_r($sql_stat,true)); + $sql_contents = $za->getStream($sql_stat['name']); // maybe copy *THIS* thing? + + // OKAY, now that we *found* the sql file if we're doing just the guess-prefix thing, we can do that *HERE* I think? + if ($this->option('sanitize-guess-prefix')) { + $prefix = SQLStreamer::guess_prefix($sql_contents); + $this->line($prefix); + return $this->info("Re-run this command with '--sanitize-with-prefix=".$prefix."' to see an attempt to sanitze your SQL."); + } + //how to invoke the restore? $pipes = []; @@ -428,9 +395,9 @@ class RestoreFromBackup extends Command //$sql_contents = fopen($sqlfiles[0], "r"); //NOPE! This isn't a real file yet, silly-billy! - $sql_stat = $za->statIndex($sqlfile_indices[0]); - //$this->info("SQL Stat is: ".print_r($sql_stat,true)); - $sql_contents = $za->getStream($sql_stat['name']); + // FIXME - this feels like it wants to go somewhere else? + // and it doesn't seem 'right' - if you can't get a stream to the .sql file, + // why do we care what's happening with pipes and stdout and stderr?! if ($sql_contents === false) { $stdout = fgets($pipes[1]); $this->info($stdout); @@ -439,15 +406,12 @@ class RestoreFromBackup extends Command return false; } - if (! self::$prefix ) { - $sql_prefix_sniffer = $za->getStream($sql_stat['name']); - self::$prefix = SQLStreamer::guess_prefix($sql_prefix_sniffer); - $this->info("Guessed prefix: '".self::$prefix."'"); - } try { - if ( $this->option('no-sanitize')) { - while (($buffer = fgets($sql_contents, self::$buffer_size)) !== false) { + if ( $this->option('sanitize-with-prefix') === null) { + // "Legacy" direct-piping + $bytes_read = 0; + while (($buffer = fgets($sql_contents, SQLStreamer::$buffer_size)) !== false) { $bytes_read += strlen($buffer); // \Log::debug("Buffer is: '$buffer'"); $bytes_written = fwrite($pipes[0], $buffer); @@ -457,7 +421,7 @@ class RestoreFromBackup extends Command } } } else { - $sql_importer = new SQLStreamer($sql_contents, $pipes[0], self::$prefix); + $sql_importer = new SQLStreamer($sql_contents, $pipes[0], $this->option('sanitize-with-prefix')); $bytes_read = $sql_importer->line_aware_piping(); } } catch (\Exception $e) { @@ -502,7 +466,7 @@ class RestoreFromBackup extends Command $fp = $za->getStream($ugly_file_name); //$this->info("Weird problem, here are file details? ".print_r($file_details,true)); $migrated_file = fopen($file_details['dest'].'/'.basename($pretty_file_name), 'w'); - while (($buffer = fgets($fp, self::$buffer_size)) !== false) { + while (($buffer = fgets($fp, SQLStreamer::$buffer_size)) !== false) { fwrite($migrated_file, $buffer); } fclose($migrated_file);