- Issue created by @swirt
- Merge request !11786#3518344 Prevent javascript asset version disclosure. โ (Closed) created by swirt
- Merge request !11787#3518344 Prevent javascript version disclosure D10.4 โ (Open) created by swirt
- ๐บ๐ธUnited States swirt Florida
If there is interest in this actually being merged, let me know and I will rework the failing asset tests to account for this change.
I don't see version numbers on assets with or without aggregation enabled.
Yes, they are there on the JavaScript files if aggregation is off. Why is aggregation off on the site?
- ๐บ๐ธUnited States swirt Florida
@cliefen Good question. Two different answers:
- Even with javascript aggregation enabled, when you hit update.php or install.php, it seems they do not use aggregation and the individual script tags are present, which is where the versions are visible.
- We have aggregation enabled on our stage an prod environments, so on all pages other than update.php or install.php it is not a full problem there. However we do have aggregation off on our dev environment so that is easier to debug js issues. And on our gov't projects all three environments are scanned due to Zero Trust mindset.
It is nice that aggregations solves it for most requests, and the Drupal recommended approach is to use aggregation, I don't think it should be a security requirement. There is nothing on the performance UI that says "use JS aggregation or your site will be more at risk."
- ๐บ๐ธUnited States swirt Florida
I updated the steps to reproduce to be more specific about when and where they are exposed.
Heads up - a move in the opposite direction is being asked for in ๐ Use library version as query strings for external css (like js does) Active .
- ๐บ๐ธUnited States swirt Florida
LOL. Interesting. I don't think that would security risk though as CSS rarely has exploitable issues. At least not that I am aware of. :shrug:
- ๐ฌ๐งUnited Kingdom catch
I think the scanner here is unreasonable, especially when talking about a dev site in a development mode, but also it would be easy and still correct to use a hash here. Should probably be the very shortest xxhash available.
@swirt if you had a situation with Bootstrap, for example, seeing its CSS version would reveal its JS version. Thatโs all on the assumption that this is a security improvement which Iโm doubtful about. In fact, donโt these libraries declare their versions within their code, defeating this proposed protection?
- ๐บ๐ธUnited States swirt Florida
@cliefen, These are all good questions, and I do not pretend to be a security expert but my current thinking on it is this:
Thatโs all on the assumption that this is a security improvement which Iโm doubtful about.
Version disclosure is labeled as a low risk but is serious enough that Drupal does not reveal its minor version, Only the major.
<meta name="generator" content="Drupal 7 (https://www.drupal.org)" /> <meta name="Generator" content="Drupal 10 (https://www.drupal.org)" />
If it is serious enough that Drupal does not reveal itself, then Drupal should probably show the same level of care to libraries and not reveal their specific version.
In fact, donโt these libraries declare their versions within their code, defeating this proposed protection?
Many do. Example: jQuery.fn.jQuery will reveal its own version. If the library wants to reveal its own version, that is the library's choice. However, I think that should remain the choice of the library's maintainers. Drupal should not be the library's loose lipped friend that blabs the secret. I think this falls into the same reason why we routinely make the composer.lock and composer.json of our sites return an access denied.
- ๐บ๐ธUnited States swirt Florida
@catch I took your suggestion on this MR and made it use a shorter hash https://git.drupalcode.org/project/drupal/-/merge_requests/11787
DAST tools and bad actors could additionally hash file contents and compare that against widely published lists to know an asset version. So, no, it actually wouldn't matter whether or not a given library includes its version in code. Anyone can still determine the version. I just want to be clear about how much this change actually would accomplish.
But, just to be clear, if a fast hash algo gets the job done, I don't see any reason not to do this.
- ๐บ๐ธUnited States swirt Florida
Yes, makes sense. This alone does very little. But does a little. I will work on fixing the failing tests to get this completed.
- Merge request !12122Resolve #3518344 "Javascript version disclosure 11.x" โ (Open) created by swirt
- ๐บ๐ธUnited States swirt Florida
I think the Merge request !12122 for 11.x is ready for review. See QA steps in previous comment.
- ๐ฎ๐ณIndia JatinGupta40
Hello,
I have tested the MR #25 and it is working absolutely fine.
Before -
After -
Network Tab -
- ๐บ๐ธUnited States dmundra Eugene, OR
Reviewing MR https://git.drupalcode.org/project/drupal/-/merge_requests/12122 and seeing that on install.php https://mr12122-4loygljzig4n8abvsdn1mltxwszyebvg.tugboatqa.com/core/inst... of the tugboat site the versions are hashed.
- ๐ซ๐ทFrance nod_ Lille
This changes the query parameter from
v=
tohv=
, it shouldn't impact much things but I'd like an other review to make sure we don't have too much things depending on the query string name. - ๐บ๐ธUnited States swirt Florida
@nod_ That is a good concern. I grepped the Drupal core codebase for
- $_GET['v'] - no results
- $_GET["v"] - no results
I also examined the 22 instances of 'parse_str(' to see if anything was looking for a query param of 'v' . Found nothing
Same for the two instances of 'PHP_URL_QUERY'I believe that nothing other than our tests (already altered for this merge request) are looking for a 'v' query parameter. My belief is based on the fact that ?v= does not exists with aggregation turned on other than a few special pages like update.php and install.php and they are not using the ?v param.
- ๐ฌ๐งUnited Kingdom longwave UK
To me this is won't fix, this is security by obscurity. It is trivial to find out the version of jQuery because it's embedded in the file anyway, and for any given library an attacker can just read the file themselves and fingerprint it to find out the version. To me it's more useful to actually keep the version number because it is occasionally helpful to developers to know which version of a JS library is in use.
- ๐ซ๐ทFrance nod_ Lille
Thanks for all the work on the issue, after a discussion with @catch and @longwave we're not going to make this change.
We don't consider the security scanner report as relevant here. Additionally it seems it's only concerned about jQuery version specifically. We have a history of backporting some security fixes to an older jQuery version to prevent core from being exposed (so a site would use a "vulnerable" version, but with our backport it wouldn't be possible to exploit that particular vulnerability). We're usually on top of it regarding jQuery.
Closing and crediting since you've been a great contributor on this issue :)
- ๐บ๐ธUnited States swirt Florida
I appreciate that nod_ thank you. One last way to think about this, my final pitch I promise ;)
As longwave points out, Security through obscurity is not the best... by itself. It is the equivalent of me hiding my house key under the flower pot, but it is still better than leaving it in the door.
An old timey phrase of "Loose lips sink ships", or the more modern "Snitches get stiches" seem to apply.
What if the shoe was on the other foot?
Drupal intentionally obscures its own version
``
What if some third party library decided to output "Drupal Version 10.36" because running internally it can detect the version. Would we want some other library disclosing that for us. Some might even find it helpful for development. I am pretty sure that leak would get closed in a hurry.Why is our own obscurity fine, but we don't show the same care with third party code.
Drupal is popular in government, but so is this particular scanner. The amount of time people like me spend responding to the monthly scans is more than I enjoy. The "higher ups" don't always understand enough to say "this finding is legit", or "this finding is silly" They pay for the service and they want to trust its findings. Then I have to say, "it is not really anything to worry about"... then they search for "version disclosure" and they find pages like this and pretty soon they are starting to second guess my professionalism, because I told them, it was no big deal. I'd prefer if the product I love (Drupal) did not put me in that situation.
Additionally it seems it's only concerned about jQuery version specifically.
This is not specific to Jquery. The scanner triggers on any third party library where a version is displayed. For any given month there would be ~5 libraries it would include in its findings. I use the patch from this issue and they all go away.
FWIW you can delete install.php and update.php in production deploys and enable aggregation. Those actions will eliminate the scanner detection.
- ๐ซ๐ทFrance nod_ Lille
This could be fixed from contrib, you can change the version from a hook_js_alter:
#[Hook('js_alter')] public function jsAlter(&$javascript, AttachedAssetsInterface $assets, LanguageInterface $language): void { foreach ($javascript as &$item) { if (!empty($item['version']) && $item['version'] !== -1) { $item['version'] = hash('xxh32', $item['version']); } }
would achieve the same and would keep people off your back :)
- ๐บ๐ธUnited States swirt Florida
These are all good solutions,
- using the patch from this issue
- deleting intall.php and update.php with aggregation on prod environments
- adding custom hook js alter.
any of which are no problem for me, a semi-experienced developer to implement. I am not sure if that holds true for the thousands of Drupal CMS adopters coming down the river.
- ๐ฌ๐งUnited Kingdom longwave UK
@swirt as @nod_ suggests this could be built as an opinionated contrib module which could be installed with no configuration, to hide the JS versions and the generator tag and anything else that you feel needs to be obscured; this just isn't something we are interested in supporting in core.
- ๐ฌ๐งUnited Kingdom longwave UK
In fact there already is contrib to hide the generator tag: https://www.drupal.org/project/remove_generator โ
So a companion module to hide JS versions makes sense to me.
- ๐ซ๐ทFrance nod_ Lille
I was thinking a contrib module would be a good place to do that, yes.
Is there a security track for Drupal CMS? if there is and there is already a module being used for it might be a good idea to submit a patch to implement those few lines. Or make a module and ask for inclusion in Drupal CMS. Core and CMS have different audiences so it might be more appropriate there
- ๐ฌ๐งUnited Kingdom catch
Drupal intentionally obscures its own version
This is advertising the version, not obscuring it.
If a site is on Drupal 8 or 9, then you can find that out. I believe the meta tag is primarily so that crawlers can aggregate Drupal versions by site, and we want them to do this by major rather than patch or minor version. For example if there was version information on https://lookerstudio.google.com/u/0/reporting/55bc8fad-44c2-4280-aa0b-5f... (which there isn't, but if there was...)
- ๐ฌ๐งUnited Kingdom catch
@cilefen Drupal CMS 1.0 was organised by tracks for different areas of functionality with different track leads. I don't think there are functional tracks any more but the list is at ๐ [META] Starshot work tracks Active .
- ๐บ๐ธUnited States swirt Florida
This is advertising the version, not obscuring it.
Sorry for the confusion. I meant that it only shows "Drupal 10" and not "10.3.7" it only shows major version, not specific version.
- ๐บ๐ธUnited States swirt Florida
I will create a new module that is no problem.
I can speak to the rationale for avoiding "Version disclosure" in the project page. That is documented a bunch of places. But what should I say for the choice made here to preserve version disclosure as a feature, not a bug. Is it really just "It is occasionally helpful to developers to know which version of a JS library is in use." or is there a better reason?
- ๐ฌ๐งUnited Kingdom longwave UK
No, the primary reason is that it's security through obscurity: hiding version numbers achieves nothing. Almost all attacks are automated, most attackers won't even check the version number and will blindly try anyway, and those that do check will fingerprint the file, rather than interpreting the query string.
- ๐บ๐ธUnited States swirt Florida
Closing this out with the new module Prevent Version Disclosure โ
Thanks nod_ for the push in that direction. That way I don't have to keep re-rolling this patch for our use.