Skip to content

escape exec commands#5673

Merged
kodiakhq[bot] merged 2 commits intobugfixfrom
escape-exec-commands
Mar 31, 2023
Merged

escape exec commands#5673
kodiakhq[bot] merged 2 commits intobugfixfrom
escape-exec-commands

Conversation

@gharlan
Copy link
Copy Markdown
Member

@gharlan gharlan commented Mar 30, 2023

No description provided.

@gharlan gharlan added this to the REDAXO 5.15.2 milestone Mar 30, 2023
@gharlan gharlan requested a review from staabm March 30, 2023 21:14
$addColor = '' != $color ? ' -background ' . escapeshellarg($color) . ' -flatten' : '';

$cmd = $convertPath . ' -density '.$density.' "' . $fromPath . '[0]" ' . $addColor . ' -colorspace RGB "' . $toPath . '"';
$cmd = $convertPath . ' -density '.$density.' ' . escapeshellarg($fromPath.'[0]') . ' ' . $addColor . ' -colorspace RGB ' . escapeshellarg($toPath);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Die Stelle hier wurde uns gemeldet als "Remote Code Execution", da der Color-Param im Backend eingegeben wird.
Ich sehe hier aber kein wirkliches Sicherheitsrisiko, da auf die Backend-Page nur Admins kommen, und Admins sowieso beliebigen Code ausführen können über das Backend.

Copy link
Copy Markdown
Member

@staabm staabm Mar 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

man könnte sagen, dass eine sql injection sicherheitslücke im frontend ausreicht um diesen parameter zu verändern und somit dann arbitrary/remote code execution auf dem server durchzuführen (also nicht nur php code wie bei eingabe/ausgabe von modulen, sondern dann auch echte shell commands)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wenn man die Templates/Module ändern kann, kann man dort aber ja genauso exec-Aufrufe einschleusen.

Copy link
Copy Markdown
Member

@staabm staabm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@gharlan gharlan added the automerge Automatisch PR rebasen und mergen label Mar 31, 2023
@kodiakhq kodiakhq bot merged commit 1bf23f0 into bugfix Mar 31, 2023
@kodiakhq kodiakhq bot deleted the escape-exec-commands branch March 31, 2023 08:03
@rex-bot rex-bot removed the automerge Automatisch PR rebasen und mergen label Mar 31, 2023
@gharlan gharlan modified the milestones: REDAXO 5.15.2, REDAXO 5.16 Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

3 participants