**List of steps to reproduce** (step by step, including full links if applicable):
* Set in LocalSettings.php:
```
$wgUseImageMagick = false;
$wgCustomConvertCommand = "gm convert %s -resize %wx%h %d";
```
The `$wgCustomConvertCommand` command is the example from https://www.mediawiki.org/wiki/Manual:$wgCustomConvertCommand.
* Upload an image file big enough to require thumbnailing (I used 600×600). Give it the destination filename `Test%w.png`.
**What happens?**:
See error message in the **Thumbnail** column:
> Error creating thumbnail: gm convert: Unable to open file (/var/www/mediawiki/w/images/8/84/Test120.png) [No such file or directory].
See in `$wgDebugLogFile`:
```
[thumbnail] thumbnail failed on <hostname>: error 1 "gm convert: Unable to open file (/var/www/mediawiki/w/images/8/84/Test120.png) [No such file or directory]." from "gm convert '/var/www/mediawiki/w/images/8/84/Test'120'.png' -resize '120'x'120' '/tmp/transform_4f4696c1492a.png'"
```
**What should have happened instead?**:
A thumbnail should have been created in /var/www/mediawiki/w/images/thumb/8/84/Test%w.png/120px-Test%w.png.
**Software version (if not a Wikimedia wiki), browser information, screenshots, other information, etc.**:
MediaWiki 1.37.2
`$wgCustomConvertCommand` is supposed to have the magic strings `%d`, `%s`, `%w`, and `%h` replaced by the source path, destination path, width, and height, respectively. What's going wrong is that `BitmapHandler::transformCustom` does the replacements //sequentially//, not simultaneously. `%s` sets replaced with a source path that contains `%w`. Then, that newly introduced `%w` is replaced with the width (in addition to the `%w` that was already present in `$wgCustomConvertCommand`).
https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/core/+/df1a7bf546886209b72542488bf50d4dc7f142b5/includes/media/BitmapHandler.php#390
Something like this may work better:
```
$cmd = $customConvertCommand;
$src = Shell::escape( $params['srcPath'] );
$dst = Shell::escape( $params['dstPath'] );
$w = Shell::escape( $params['physicalWidth'] );
$h = Shell::escape( $params['physicalHeight'] );
$cmd = preg_replace_callback('/%[dswh]/',
function ($m) use ($dst, $src, $w, $h) {
return [
'%d' => $dst,
'%s' => $src,
'%w' => $w,
'%h' => $h,
][$m[0]];
},
$cmd
);
```
I did not find any security vulnerabilities caused by this bug. The only substitution an attacker can control flexibly is `%s`, and the `%w` and `%h` substitutions that happen later cannot contain shell metacharacters because they are decimal integers. But the pattern of sequential substitution is fragile (in the debug log above, observe how the `120` is //outside// the shell quoting), and could give rise to shell injection vulnerabilities if other code were to change. For example, if `File::makeTransformTmpFile` were to use the basename of the source file in its temporary destination filename, then uploading a file with a name like `%s;echo hacked;echo .png` would result in shell execution. The same could happen if, in the future, another substitution string other than the four already supported were added.