Thinking about how we handle existing user accounts if we change the hashing behaviour...
If we start to enforce a limit on the password length, we'd need to decide whether we try to do something helpful / graceful in the UI e.g. tell users that supply a longer input that they should only provide the first 72 bytes of their password, or try to do that for them automatically (and tell the user this is what's happening?).
That may be easier said than done if e.g. the password includes multi-byte characters and the 72 byte threshold falls in the middle of a character. The maxlength attribute "is measured in UTF-16 code units, which is often but not always equal to the number of characters", but I'm not certain what that would mean in all cases where existing passwords exceed the limit in bytes.
I'd think we'd certainly want to avoid e.g. enforcing a limit in the login form which effectively makes it impossible for users with passwords that exceed the limit to authenticate (perhaps they'd be forced to do a password reset - but we can't know that's always available on all Drupal sites?)
If we start pre-hashing passwords (would we do that universally or only when the input is longer than a threshold?), we'd perhaps need to re-generate the stored password hash for existing user accounts (this has been done before in Drupal when the hashing algorithm changed).
Presumably that means:
- A user with a >72 byte password would be successfully authenticated based on the first 72 bytes of the password they supply matching the existing hash.
- We then generate a new hash based on the full password they've supplied and store that for future use.
- The password they've given may or may not match what they originally gave - we can only ever know that the first 72 bytes matched.
- From then onwards, the password they give would need to fully match the one used to generate the new hash.
No way around that AFAICS, but it's pretty ugly; doesn't change the fact we're only considering the first 72 bytes at the moment anyway.
Need to verify whether this (still) actually applies, but here's a strong argument not to pre-hash (or at least to approach doing so with extreme caution):
https://blog.ircmaxell.com/2015/03/security-issue-combining-bcrypt-with....
There's an argument that it's best not to use just a plain hash in the pre- step before bcrypt - e.g. instead of:
bcrypt(sha512($password))
..you'd do:
bcrypt(sha512($salt + $password))
..or use a "pepper" instead of salt, or use a HMAC.
Some references:
For context, discussion around the same thing in Laravel:
https://securinglaravel.com/security-tip-limiting-bcrypt-passwords-to-72...
In Laravel 12 they added a configurable flag to have the Hash class throw an exception when given input longer than the hashing algorithm properly supports.
There's some follow-up discussion about what approach to take in future e.g. adopting a different hash algo. by default.
benjifisher → credited mcdruid → .
Verified that the test-only job fails as expected:
https://git.drupalcode.org/issue/drupal-3526769/-/jobs/5685661
..but tests pass with the changes.
Here's the contents of the test tarball:
core/modules/config/tests/fixtures$ tar ztvf not_just_config.tar.gz
-rw-rw-r-- mcdruid/mcdruid 48 2025-06-26 10:50 config.one.yml
-rw-rw-r-- mcdruid/mcdruid 48 2025-06-26 10:50 config.two.yml
-rwxrwxr-x mcdruid/mcdruid 48 2025-06-26 11:03 executable.yml
-rwxrwxr-x mcdruid/mcdruid 60 2025-06-26 10:52 script.sh
I was wondering about putting that in a comment in the test to save anyone having to examine it themselves.. but perhaps that just risks getting out of sync with the fixture file, and might not be necessary / helpful anyway.
Added a basic test that verifies a shell script cannot be smuggled into a config import tarball, and checks that files are not executable after imports (even when they are inside the tarball).
I think this needs to be a functional test, as the changes being made are in the processing of the import form on submission.
Updates to IS now that we've agreed a proposed solution.
Thank you to the Usability Team who reviewed and discussed this issue in their meeting 📌 Drupal Usability Meeting 2025-05-30 Active . I believe it's still on the todo list to comment here, but the meeting recording and transcript are linked to from that issue so we can glean details of their review in the meantime.
To attempt to summarise - the question was:
What does the UX-Team think about having mandatory stripping of special chars from filenames, plus enabling swapping spaces out for - or _ by default?
...and the answer:
benji: okay, there's a rather long list of special characters that are hard coded.
benji: ... You can't remove anything from that list. It's it's hard coded. The only thing you can change in configuration is whether to replace spaces.
benji: ... the main usability question is whether to allow people to override it.
benji: ... I think I agree that we should
benji: use the more secure default. So by default, replace spaces.
benji: Let let people override. And Ralph agrees.
benji: So we're unanimous on that.
Ralf Koller: [T]hat looks like a reasonable change, definitely.
I'll ask @benjifisher to correct me if that's not quite accurate.
Another important issue that was brought up was whether we're proposing to retrospectively tackle existing files.
I think that's a "no"; the likelihood of unintended consequences is really high if we tried to rename existing files, and we should limit the scope to future uploads only. (This is what Benji and the team correctly assumed).
So let's proceed on the basis that we want to:
- Introduce mandatory replacement / removal of "dangerous characters", and..
- Enable replacement of spaces in filenames by default, but allow this to be overridden.
Benji mentioned in the UX meeting that any overrides may have to happen only in settings.php as otherwise an attacker can potentially make those changes themselves (e.g. via XSS in the UI).
That's definitely a good point, but I think in the case of allowing spaces in filenames I'd be comfortable allowing that change to made in the UI if the dangerous characters are always removed with no way of overriding that behaviour.
benjifisher → credited mcdruid → .
benjifisher → credited mcdruid → .
Existing tests pass with the changes in the MR.
We should add one or more tests that show:
- Only .yml files are extracted from the tarball.
- A file with executable permissions in the tarball is extracted with default (664) permissions into the sync directory.
Looks like it's legitimate to have subdirectories within the tarball, based on \Drupal\Tests\config\Functional\ConfigExportImportUITest::testExportImportCollections.
I'd initially assumed there was only ever a flat list of yml files.
Perhaps the module could also do a stable release and opt in to security coverage :)
Thanks for the very quick response @steven jones
+1 to concerns raised in #26 / #27.
Allowing the path to an executable to be set in the UI is a really valuable way for an attacker to escalate XSS to RCE.
One recent related issue was adding Symfony Mailer transports to core 📌 [PP-1] Add symfony mailer transports to DIC Postponed where I believe it's necessary to set anything non-standard via settings.php.
Validation is really important (noting the value is being checked on submit with is_executable() and there are tests showing that command -args type values will fail).
I agree that this makes ✨ Require password for certain admin actions Active a high priority security hardening.
benjifisher → credited mcdruid → .
benjifisher → credited mcdruid → .
In the meantime I've submitted a PR to guzzle with a fairly crude way of minimising the impact of the Gadget Chain:
https://github.com/guzzle/guzzle/pull/3286
We'll see how that goes, but I don't think it's a reason to stop looking at dropping the class from Drupal.
benjifisher → credited mcdruid → .
I've posted some more details about my motivation for pursuing this change in a blog post about a recent Security Advisory:
https://www.mcdruid.co.uk/article/hacking-ai-module-drupal-cms
Hopefully the extra context this provides outweighs any whiff of self-promotion :)
To summarise the proposal briefly, the change would be:
- Filenames always have dangerous characters stripped (or replaced with an inert character like
-). In practice this means most but not all punctuation (e.g. see list in #8). - Filenames have spaces replaced by
-. This is existing optional functionality which would be enabled by default. - The replacement character used in both cases could be configurable (again, that functionality already exists).
This would be pretty similar to how filenames are sanitised by both WordPress and Typo3.
Thanks - I'm going to tag this for a usability review (per https://www.drupal.org/docs/develop/issues/issue-procedures-and-etiquett... → ) as I agree that it's likely stripping spaces from filenames on uploads may not be universally welcomed.
It would, however, have benefits in terms of Security Hardening (it's very hard to achieve Command Injection without any spaces). (Acknowledging the comment in #20 that if a command injection vulnerability of similar exists, this is not the only fix that's needed.)
There are also other benefits like making it easier to iterate over batches of files in the shell etc.. but that's probably out of scope here.
I personally loathe spaces in filenames but I am probably not representative of a broad cross section of users.
So far I think there's some consensus that stripping "special characters" from filenames likely provides enough benefit (in terms of security) to outweigh annoyance some users may feel at having e.g. punctuation marks removed from their filenames, but stripping spaces may be seen as a step too far.
It could be a default that site owners could disable in the file handling options. I'd argue to make the special character stripping mandatory, but that's also open to debate.
Previous comments include some light research on how other CMSs/Frameworks do this. Wordpress and Typo3 both strip (or substitute) most punctuation by default, and it looks like they both swap out spaces for either an underscore or dash AFAICS.
Drupal already has an option to replace whitespace in filenames with _ or - but it's not enabled by default. I'd like to turn that on by default as part of this issue.
What does the UX-Team think about having mandatory stripping of special chars from filenames, plus enabling swapping spaces out for - or _ by default?
(I've not yet updated the IS to eliminate any proposed options as I don't think we have eliminated any yet.. agree we should do that when we get to that point though.)
Adding REMOTE_ADDR as a trusted proxy may work in some cases, but it can also cause problems such as making it possible to spoof the client IP via X-Forwarded-For (per #5 and the existing cautionary note in the comments).
benjifisher → credited mcdruid → .
Great, thanks for the nudge in the right direction.
Tests passing.
One issue is that comments in the test refer to the gadget chain as "Drupal/FI1" but there's currently no such thing in https://github.com/ambionics/phpggc (a new PR may be opened for it soon though).
I think this should be back at NR.
There is a linting error / warning in the MR but I don't understand what it's saying sufficiently to know whether it's related to this change - it doesn't appear to be at a glance.
Very definitely +1 to the idea of requiring an additional authentication step to install a module via Project Browser to defend against XSS/CSRF attacks.
Benji makes good points about SSO and TFA, and whether requiring the password to be supplied is overly prescriptive; having an API for (re-)authentication would be great.
I can think of other use cases for this too (I'll link to issues I plan to file).
As for being able to toggle the extra protection on/off (e.g. development mode), would it make sense to have this as setting that cannot be altered via the UI?
greggles → credited mcdruid → .
sorry, didn't mean to change status
Thanks, that's a good start. Looks like you've borrowed the list of chars to strip from wordpress. Perhaps we could remove a few of the "safe" ones based on the list in #7; we're only talking about a few e.g. % + = :.
FWIW here's how typo3 does this:
https://github.com/TYPO3-CMS/core/blob/v13.4.9/Classes/Resource/Driver/L...
The comments suggest that it's pretty strict:
... any character not matching [.a-zA-Z0-9_-] is substituted by '_'
...but in actual fact it's a bit more nuanced as there are some settings around allowing / remapping utf8 characters.
I'll try to do some work on this but am short on time just now.
FWIW by default Wordpress removes a load of "special" characters from filenames:
https://github.com/WordPress/wordpress-develop/blob/6.7.2/src/wp-include...
function sanitize_file_name( $filename ) {
...
$special_chars = array( '?', '[', ']', '/', '\\', '=', '<', '>', ':', ';', ',', "'", '"', '&', '$', '#', '*', '(', ')', '|', '~', '`', '!', '{', '}', '%', '+', '’', '«', '»', '”', '“', chr( 0 ) );
...
$filename = str_replace( $special_chars, '', $filename );
Joomla seems to have decided to let the underlying OS decide what it will and will not allow in filenames :shrug:
Having said that, I went to look for a definitive list of "dangerous characters" and ended up with quite a lot of "it depends".
This is not definitive, but e.g. https://stackoverflow.com/questions/15783701/which-characters-need-to-be...
There are some quite good lists using e.g. the shell escaping option that printf (and apparently ls) offer - e.g.
No, character % does not need to be escaped
No, character + does not need to be escaped
No, character - does not need to be escaped
No, character . does not need to be escaped
No, character / does not need to be escaped
No, character : does not need to be escaped
No, character = does not need to be escaped
No, character @ does not need to be escaped
No, character _ does not need to be escaped
Yes, character needs to be escaped
Yes, character ! needs to be escaped
Yes, character " needs to be escaped
Yes, character # needs to be escaped
Yes, character $ needs to be escaped
Yes, character & needs to be escaped
Yes, character ' needs to be escaped
Yes, character ( needs to be escaped
Yes, character ) needs to be escaped
Yes, character * needs to be escaped
Yes, character , needs to be escaped
Yes, character ; needs to be escaped
Yes, character < needs to be escaped
Yes, character > needs to be escaped
Yes, character ? needs to be escaped
Yes, character [ needs to be escaped
Yes, character \ needs to be escaped
Yes, character ] needs to be escaped
Yes, character ^ needs to be escaped
Yes, character ` needs to be escaped
Yes, character { needs to be escaped
Yes, character | needs to be escaped
Yes, character } needs to be escaped
I feel like most of those make sense; and there are a fair number of characters that would be allowed.
Whether this would be more or less confusing / user friendly than just stripping all punctuation though is perhaps a usability question.
Looks like the code mostly uses regex; I wonder whether the punct character class would be any use. It'd remove the "safe" characters from the list above which we may not want to do.
To be clear what I'm looking for here is something that would remove all the dangerous shell characters but not limit users to strict ASCII alphanum without e.g. accented letters etc..
@kim.pepper although the existing options would remove all of the characters I highlighted as a concern, they do more than that.
I wouldn't advocate stripping filenames down to only ASCII / alpha-numeric characters (by default or as a mandatory setting) because of how restrictive that is for different languages.
I'm proposing a fairly limited number of forbidden characters - those which have significance for the shell - which would be a subset of what would be stripped by any of the existing options IIUC?
I'd recommend that we enforce the removal (/ substitution) of the most dangerous characters.
For removal of spaces, that'd be ok as a default (would people really want to turn it off?!)
greggles → credited mcdruid → .
Ok, thanks for confirming.
I think 1.0.x is still marked as the default at https://git.drupalcode.org/project/cms/ FWIW.
benjifisher → credited mcdruid → .
Is this a duplicate of 📌 Add debug mode (and fix virus name for executable) Needs work (which has an MR)?
I've verified that if we inject something like an XSS payload into the scanner output, it gets logged verbatim.
For example (this is a low effort quick hacky test):
+++ b/src/Scanner/Executable.php
@@ -65,6 +65,9 @@ class Executable implements ScannerInterface {
// values greater than 2. Any value of 2 or greater means that the scan
// failed, and the file has not been checked.
exec($cmd, $output, $return_code);
+
+ $output[] = '<script>alert("oh noes!")</script>';
+
$this->_output = implode("\n", $output);
An example log entry in the db looks like:
> SELECT * FROM watchdog \G
*************************** 78. row ***************************
wid: 78
uid: 1
type: Clam AV
message: Virus %virusname detected in uploaded file %filename. Debug output: %output
variables: a:3:{s:9:"%filename";s:14:"/tmp/phpALzyP1";s:10:"%virusname";s:15:"Eicar-Signature";s:7:"%output";s:361:"/tmp/phpALzyP1: Eicar-Signature FOUND
----------- SCAN SUMMARY -----------
Known viruses: 8704988
Engine version: 1.0.7
Scanned directories: 0
Scanned files: 1
Infected files: 1
Data scanned: 0.00 MB
Data read: 0.00 MB (ratio 0.00:1)
Time: 10.241 sec (0 m 10 s)
Start Date: 2025:03:14 14:01:59
End Date: 2025:03:14 14:02:09
<script>alert("oh noes!")</script>";}
severity: 3
The traditional approach in Drupal is to filter on output ( https://www.drupal.org/docs/develop/security/why-does-drupal-filter-on-o... → ). The dblog admin UI escapes this appropriately.
We could pass the debug output through a filter before logging it.. but I'm not sure that's necessary, or best practice.
We don't want to fill syslog log entries with lots of html-encoded characters unnecessarily, for example (I also don't think the typical output of the clam scanner will contain anything bad.. especially as - per my earlier comment - the user-chosen filename is not currently included in these logs).
So I think this MR's ready for review.
Same deal for connection to the daemon by socket - I've added the same scanner output as for the tcp/ip mode.
This is a bit duplicative.. but for now I've added the same message about the failed connection to the daemon to the $_output property so that it gets logged as the scanner output when debug is enabled.
I suppose we could omit the other log entry when debug is enabled, but I'm not certain that's a good idea; it might already be used for monitoring etc..
Daemon mode example log entries (again, verbose + debug enabled):
Uploaded file /tmp/phpuLzC2k checked and found clean. Debug output: stream: OK
Virus Eicar-Signature detected in uploaded file /tmp/phpnGu0G4. Debug output: stream: Eicar-Signature FOUND
...and if I tried to deliberately break the daemon set up..
Uploaded file /tmp/phpkCRZnu could not be checked, and was deleted. Debug output:
Hmm that's not so useful. In that case the daemon wasn't actually listening for connections at all. Would probably be useful to try to log something vaguely informative in that situation.
If I try to break the config of the daemon it just doesn't start up.. so I think not being able to connect is one of the more realistic problems.
The module does actually log messages when it can't connect:
https://git.drupalcode.org/project/clamav/-/blob/2.1.x/src/Scanner/Daemo...
Unable to connect to ClamAV TCP/IP daemon on clamav:3310
Manually tested with executable mode; seems to work as expected.
Example log message when the scanner found a problem (with both verbose and debug enabled):
Virus Eicar-Signature detected in uploaded file /tmp/phpqQahST. Debug output: /tmp/phpqQahST: Eicar-Signature FOUND ----------- SCAN SUMMARY ----------- Known viruses: 8704988 Engine version: 1.0.7 Scanned directories: 0 Scanned files: 1 Infected files: 1 Data scanned: 0.00 MB Data read: 0.00 MB (ratio 0.00:1) Time: 10.072 sec (0 m 10 s) Start Date: 2025:03:14 11:57:30 End Date: 2025:03:14 11:57:40
It'd ideally be better if the original filename of the upload was logged instead of the tmp name, but that's a different issue.
Another example where I deliberately broke the clamscan executable - this is the sort of thing debug mode may be more useful for in reality:
Uploaded file /tmp/phpXYXjA4 could not be checked, and was deleted. Debug output: LibClamAV Error: cli_load(): Can't open file /var/lib/clamav/main.cvd LibClamAV Error: cli_loaddbdir: error loading database /var/lib/clamav/main.cvd LibClamAV Error: cli_loaddbdir: No supported database files found in /var/lib/clamav ERROR: Can't open file or directory ----------- SCAN SUMMARY ----------- Known viruses: 0 Engine version: 1.0.7 Scanned directories: 0 Scanned files: 0 Infected files: 0 Data scanned: 0.00 MB Data read: 0.00 MB (ratio 0.00:1) Time: 0.004 sec (0 m 0 s) Start Date: 2025:03:14 12:02:03 End Date: 2025:03:14 12:02:03
I'll try to do some similar tests with the daemon mode too, and socket if I have time (https://github.com/fwust/ddev-clamav looks like it'll make testing daemon mode pretty easy.. socket mode may involve a bit more manual set up).
There are quite a few linting errors between cspell / phpcs / phpstan.
A couple of them are introduced by the changes in the MR, but that's as a result of following the patterns / conventions that are already there e.g.
32 | WARNING | Property name "$_output" should not be prefixed with an underscore to indicate visibility
| | (PSR2.Classes.PropertyDeclaration.Underscore)
32 | ERROR | Class property $_output should use lowerCamel naming without underscores
| | (Drupal.NamingConventions.ValidVariableName.LowerCamelName)
...where there are other properties like $_virus_name that hit the same problems.
It would probably make sense to address all of this in a dedicated issue, and just follow the existing patterns in this MR.
Rerolled the old patch against 2.1.x and put it into an MR.
There's a todo list in #3
I don't think the _file property is an issue any more - I've removed it as it wasn't being used for anything much.
I've not tested this manually (with any of the different configurations) yet though, and the output from the scanner should be sanitised.
Moving to NR for the testing, but we shouldn't commit this without the sanitisation.
mcdruid → changed the visibility of the branch 3178741-add-debug-mode to hidden.
Switching branches to create a new MR.
Oh, this was already fixed in https://git.drupalcode.org/project/ai/-/commit/ede64f611afb54e8d6919b3ff...
mcdruid → created an issue.
greggles → credited mcdruid → .
Thanks - you're right.. but I already applied the same changes :)
Thank you for the offer - very happy to accept it!
Adding credit from the private security issue.
Tests are passing.
Noting that this test is very similar to the one in 📌 Add tests for SA-CORE-2024-006 Active so anything we want to change here will likely also apply over there.
mcdruid → changed the visibility of the branch 3508240-add-tests-for to hidden.
This was only for 10.x
Adding credit from the private security issue.
Adding credit from the private security issue.
In general we'd use expectException in a test, but in this case we want to do some more checks after the exception is thrown.. so we have a try / catch instead.
I am in two minds about having this very specific assertion about the TypeError:
$this->assertStringContainsString('Cannot assign Drupal\Component\PhpStorage\FileStorage to property Drupal\Core\Config\StorageComparer::$targetCacheStorage', $e->getMessage());
That feels a bit like it might break in future, and we wouldn't necessarily care so long as the TypeError is still thrown.
I'll leave it in for now.
Tests are passing on the MR.
greggles → credited mcdruid → .
+1
v4 is the current version - so e.g. https://nvd.nist.gov/vuln-metrics/cvss/v4-calculator
I had a look at scoring a Drupal SA recently and it seems we'll have to get used to some new concepts if we're going to use v4 such as "Vulnerable System Impact" vs "Subsequent System Impact".
There are comprehensive docs:
https://www.first.org/cvss/user-guide#Assessment-Guide
I think perhaps we should aim to start doing a CVSS (v4) score in parallel to the existing system for a while, with the aim of switching over as soon as practical.
I think the problem is here:
https://git.drupalcode.org/project/securitydrupalorg/-/blob/7.x-1.x/feat...
$handler->display->display_options['filters']['field_issue_status_value']['expose']['remember_roles'] = array(
2 => '2',
3 => 0,
1 => 0,
9 => 0,
18 => 0,
23 => 0,
5 => 0,
4 => 0,
);
Not sure if the status values have changes since this view was exported to code, but looks like the default selected value of "2" doesn't match the db any more?
Do we need to change this to:
$handler->display->display_options['filters']['field_issue_status_value']['expose']['remember_roles'] = array(
'Open' => 'Open',
);
(Been a while since I looked at one of these - perhaps it needs to be numerical ids that are not reflected in the UI.. don't remember).
I'm not certain if the view has been overridden and therefore changes to this file wouldn't actually make any difference, so I won't create a patch/MR - I am happy to if that would help though.
The docs → currently say:
Project moderators will not add as owner, maintainer, or co-maintainer a person who cannot opt projects into security coverage
Only a project owner (/ existing maintainer) can add a user without the opt-in to security coverage attribute as a co-maintainer.
The docs also suggest that users interested in being added as a co-maintainer...
File an issue in the project's queue. Use Support request as the category and state your interest in becoming maintainer or co-maintainer. Adding links to issues for the relevant project in which you have added merge requests or patches or where you have reviewed merge requests or patches from other users will help demonstrate that you are actively involved in the project. Describe the motivation behind your request.
We could change the wording here a little to move from "should" to "must"; rather than "providing some examples of your contributions might help the application" we could mandate that such examples are provided, possibly specifying a threshold (e.g. minimum 5 examples).
We could also provide some counter-examples of "contributions" which might not be considered as valid e.g. +1's, screenshots of patches applying etc..
All of that's fairly soft in terms of being enforceable but it would at least mean that the cases of inadequately supported applications to be added as co-maintainers we've looked at here would be in clear contravention of the stated rules, which would justify action being taken to undo the applications.
It would also help to reinforce the arguments - put forward in previous comments in this issue - that contributions can and should be made without maintainership status of a project, in order to establish some credibility and track record before a user is considered as a suitable candidate to be added as a co-maintainer.
As for whether anyone has the "right" to undo an action such as granting a user co-maintainership of a project on d.o, I'd defer to the Drupal Association and their legal advisors, but the general gist of e.g. https://www.drupal.org/docs/develop/git/setting-up-git-for-drupal/drupal... → and the other Terms of Service published for d.o., suggests to me that it's quite likely that the DA would be permitted to take whatever action (on d.o and associated systems) is deemed necessary to maintain the security of the overall Drupal project and ecosystem.
I think it would be more disruptive to the users in question here to have their accounts suspended than it would for the co-maintainership to be revoked. It's a bit of a pyrrhic victory if the users remain as maintainers but cannot access their accounts.
Personally I'd support the removal of the role in these specific cases, on the basis that the spirit of the rules was not adequately followed, and that the documentation will be improved to clarify those rules to avoid a recurrence.
benjifisher → credited mcdruid → .
The only way seckit can do anything for static assets is if Drupal's private files (or a similar stream-wrapper) is used to serve those files through Drupal / PHP.
Otherwise Drupal doesn't get involved in serving the files at all - the webserver does it without "spinning up" a Drupal / PHP process.
In the majority of cases it's not worth the extra overhead of having Drupal/PHP serve your static files; the webserver is typically really good at doing that job efficiently.
I will close this as "works as designed" as there's nothing for the project to do; this is really down to the configuration of the application and the webserver.
Drupal apps will typically try to let a webserver (e.g. apache, nginx) handle requests for static files such as images, css files etc..
You can make PHP/Drupal handle those requests (e.g. with the private files functionality) but that's usually far less efficient than letting a webserver do the job it's designed for.
That means that Drupal often isn't involved in serving those static assets at all.
If you want to emit particular security headers for them, you'll often have to configure your webserver to do that - e.g. with .htaccess rules or whatever the equivalent is for your webserver of choice.
Some security headers are less relevant when serving static files anyway.
So I think this is "works as designed" unless I'm missing something?