framework icon indicating copy to clipboard operation
framework copied to clipboard

Production mode shows some debug information

Open ghost opened this issue 7 years ago • 12 comments
trafficstars

This ticket is a revamp of #7751 as per stale bot request.

  • Vaadin Framework version 7.7.13 and also 8.4.2

  • Browser version Google Chrome 67.0.3396.62 64 bit

  • Web container name and version Apache tomcat 8.5.31, Java 1.8.172

  • Description of the bug We have several web applications done with Vaadin, paid third parties do Vulnerability Assessments and Penetration Tests on our applications in order to ensure the customers that the application is as secure as possible. We have received a complaint that it is still possible to read debug information from our webapps, even if we have set the productionMode param to true in the web.xml. The trick is to look at the browser console (further details in next bullet points).

  • Minimal reproducible example Open browser, digit the url of a running web application (which was deployed with the productionMode param set to true) and launch it. Open the browser console (F12 in Chrome) and clear the console. Now append ?debug to the url and press enter, the browser console should fill with a lot of extra information not generally visible (further details in the next bullet points).

  • Expected behavior The console should show little or no logs as much as when the ?debug param is absent.

  • Actual behavior It shows a lot of debug log information: Vaadin bootstrap loaded vaadinBootstrap.js?v=7.7.13:14 init application AppName {theme: "ThemeName", versionInfo: {…}, widgetset: "widgetsetName", widgetsetReady: true, comErrMsg: {…}, …} vaadinBootstrap.js?v=7.7.13:14 loadTheme ...themeInfos... vaadinBootstrap.js?v=7.7.13:14 load widgetset ...widgetsetInfos... vaadinBootstrap.js?v=7.7.13:14 Fetching root config vaadinBootstrap.js?v=7.7.13:14 sending request to ...urlInfos... vaadinBootstrap.js?v=7.7.13:14 Got root config response {"v-uiId":0, ...bigJSON...} vaadinBootstrap.js?v=7.7.13:14 Setting pending startup AppName vaadinBootstrap.js?v=7.7.13:14 Widgetset registered widgetsetName vaadinBootstrap.js?v=7.7.13:14 Starting from register widgetset AppName

  • Possible Remediation The VA&PT consultants suggested us to assign to the logger a void function in order to avoid logging debug information: var log = function(){};.

ghost avatar May 30 '18 14:05 ghost

What about ignoring "?debug" on production systems?

ahoehma avatar May 30 '18 16:05 ahoehma

I mean, its clearly a bug...it should be fixed once and for all.

That said, if you have any advice on how to mitigate it in a clean and java friendly way, I'm here to listen and to learn.

ghost avatar May 31 '18 09:05 ghost

What can an attacker do with this debugging information that they couldn't also do by instead going to the network tab in the same browser console and picking out the same data from the requests and responses?

Legioth avatar May 31 '18 11:05 Legioth

(I managed to test the issue also with a newer version of Vaadin and it stays the same, I have updated the ticket)

@Legioth Well, that is part of the bigger picture. That data should not be sent in the first place if is a debug only data and could also lead to information disclosure benefiting an attacker, right? This is only of one the issues that can arise security wise, for example the versions of vaadin and other components should be not traceable in production mode (I've opened #10958 for that), as it happens in other frameworks or products.

What might not have been stressed enough, is that this issue (with others from our side, of course) arose after a 8-days-long vulnerability assessment done from third party professionals hired by a customer of ours (in our line of business, it is mandatory to have a VA a year on this kind of software); so if they care for it, I need to care for it too, because next time they do a VA on our software, they will start from what they discovered previously.

What is my point of view in all of this discussion?

  • if productionMode is true, then
    1. if what is shown is really only just debug data, it should not be sent from server to client in the first place (it is also quite inefficient in my opinion); else
    2. if that data is relevant to the correct behavior of the application, then ?debug should not log in the browser console and it will at least be a bit less easy for an attacker to read the data; not impossible of course, but it should be a first step into a safer behavior.

This is the behavior that I expect and that (in my opinion) any security minded person would expect. I also think that this is clearly a bug that should be fixed as a security issue. That said, if for whatever reason is not possible for you to patch it, I would like at least a good hint on how to work it out in a clean and java friendly way.

Thanks and sorry for the long message.

ghost avatar May 31 '18 12:05 ghost

I do agree that it would be good if the version number of Vaadin wouldn't be discoverable through the browser, but this is not as easy as it sounds for a couple of reasons:

  1. The version number is used for cache busting so that web browsers will download new versions of the bootstrap script and the theme after the application has been updated to a newer version. This could be avoided by instead using a hash of the file contents as a part of the resource URI, but this would break the assumption that those static files could be served from a separate server and thus not be available to the server that generates the URIs.
  2. The version number is compiled into the widgetset that is run by the browser for debugging purposes. Avoiding this would either require that the very useful debug information wouldn't be available during development, or alternatively that there would be separate widgetset builds for production and non-production use, and that the non-production files wouldn't even be deployed as static files on the server.
  3. Even if there wouldn't be any version numbers anywhere, it's still quite easy to deduct the version number by looking at the actual JavaScript code in the widgetset. As a trivial example, one could check whether a TreeGrid implementation is included and based on that determine whether the version is before or after Vaadin 8.1.

I don't agree that security would be improved by not logging information that is already trivially available in the browser through other means such as the network inspector. On the contrary, it would only give a false sense of security while making it harder for someone to realize that the data is actually there.

The suggested "fix" of replacing console.log with a no-op implementation can very easily be circumvented by putting the original implementation back there again.

Legioth avatar May 31 '18 13:05 Legioth

@Legioth Could you copy (or even better, move) the part regarding the version in the other ticket? So I can stay on topic on both ;)

Regarding the debug information, as I stated, if it is really only debug information, it should not even come to the client side; from your answer I get that it is all information that is useful for the correct execution of the application, in that case I think it is still useful to hide it from the browser console log. To explain why, I propose a quick example: why does everyone uses a lock for his/her locker in the gym? It's pretty straight forward to crack it open by using brute force or by picking it with the appropriate tools, but it makes it less easy to open the locker and it discourages some of the possible offenders. I think this is the case with this "patch", it does not solve the problem but it is a small security enhancement that could reduce a bit the attack surface.

I agree that replacing console.log it is quite a dumb attempt, I've to say that they probably are not expert programmers, so you will find a better way to patch it.

If you still think that it is not worth for the Vaadin team to patch this bug (and, as you might have understood, I strongly disagree), I then ask you at least for some help and hints in how we could patch it ourselves, without losing the possibility to update the framework when new versions come along and in a clean way.

Thanks for the support

ghost avatar May 31 '18 14:05 ghost

If all that matters is that information that is already present in the browser isn't displayed in the browser console, then replacing console.log is just as brilliant or as dumb as changing the implementation of Vaadin's client-side to not pass the information to that method.

If instead targeting that the information wouldn't at all be present in the browser, then the discussion is only about #10958 and not directly related to the browser console. As long as #10958 isn't fully fixed, I'd even say that it's better that the existence of that information is evident through the browser console so that developers may become aware of the potential issue and make an educated decision about how critical that is for their application.

Legioth avatar Jun 01 '18 13:06 Legioth

@Legioth any suggestions on how we could apply the console.log hack in our Vaadin environment?

ghost avatar Jun 04 '18 08:06 ghost

@fabcan You can add the JavaScript snippet using the features described in https://vaadin.com/docs/v8/framework/articles/CustomizingTheStartupPageInAnApplication.html

Legioth avatar Jun 04 '18 08:06 Legioth

@Legioth thanks for the link, I've modified my custom servlet in order to disable logging on production mode, nevertheless I hope to see a patch from your side that fixes the problem from the root.

ghost avatar Jun 04 '18 13:06 ghost

Hello there!

We are sorry that this issue hasn't progressed lately. We are prioritizing issues by severity and the number of customers we expect are experiencing this and haven't gotten around to fix this issue yet.

There are a couple of things you could help to get things rolling on this issue (this is an automated message, so expect that some of these are already in use):

  • Check if the issue is still valid for the latest version. There are dozens of duplicates in our issue tracker, so it is possible that the issue is already tackled. If it appears to be fixed, close the issue, otherwise report to the issue that it is still valid.
  • Provide more details how to reproduce the issue.
  • Explain why it is important to get this issue fixed and politely draw others attention to it e.g. via the forum or social media.
  • Add a reduced test case about the issue, so it is easier for somebody to start working on a solution.
  • Try fixing the issue yourself and create a pull request that contains the test case and/or a fix for it. Handling the pull requests is the top priority for the core team.
  • If the issue is clearly a bug, use the Warranty in your Vaadin subscription to raise its priority.

Thanks again for your contributions! Even though we haven't been able to get this issue fixed, we hope you to report your findings and enhancement ideas in the future too!

stale[bot] avatar Nov 01 '18 14:11 stale[bot]

Hello there!

We are sorry that this issue hasn't progressed lately. We are prioritizing issues by severity and the number of customers we expect are experiencing this and haven't gotten around to fix this issue yet.

There are a couple of things you could help to get things rolling on this issue (this is an automated message, so expect that some of these are already in use):

  • Check if the issue is still valid for the latest version. There are dozens of duplicates in our issue tracker, so it is possible that the issue is already tackled. If it appears to be fixed, close the issue, otherwise report to the issue that it is still valid.
  • Provide more details how to reproduce the issue.
  • Explain why it is important to get this issue fixed and politely draw others attention to it e.g. via the forum or social media.
  • Add a reduced test case about the issue, so it is easier for somebody to start working on a solution.
  • Try fixing the issue yourself and create a pull request that contains the test case and/or a fix for it. Handling the pull requests is the top priority for the core team.
  • If the issue is clearly a bug, use the Warranty in your Vaadin subscription to raise its priority.

Thanks again for your contributions! Even though we haven't been able to get this issue fixed, we hope you to report your findings and enhancement ideas in the future too!

stale[bot] avatar Jan 09 '22 01:01 stale[bot]