nextgenhealthcare / connect

The swiss army knife of healthcare integration.
Other
932 stars 280 forks source link

[BUG] Cross Channel Code Injection #4474

Open MichaelLeeHobbs opened 3 years ago

MichaelLeeHobbs commented 3 years ago

Describe the bug A clear and concise description of what the bug is. Is this consistently reproducible? Yes

To Reproduce Setup steps (if required). Example:

  1. Channel A - Code Injector
    • Data Types all raw
    • Source Transformer
      String.prototype.toString = function () {
      var str = this.valueOf()
      logger.info('String.prototype.toString Code injection! data = ' + str)
      return str
      }
  2. Deploy Channel A send it a blank message
  3. Channel B - Code Injector Test
    • Data Types all raw
    • Source Transformer
      msg = msg
  4. Deploy Channel B and send it a message, for example This is a test!
  5. Check server log

Expected behavior A clear and concise description of what you expected to happen.

Each channel should be a seal sandbox with their own scope without side effects and only able to transfer data via the expect methods.

Actual behavior A clear and concise description of what actually happens.

Any given channel can pollute the global JS scope.

Screenshots If applicable, add screenshots to help explain your problem.

Environment (please complete the following information):

Workaround(s) Are there one or more workarounds for this issue currently? None.

Additional context Add any other context about the problem here.

This is not a serious issue for my organization but I could see this being a major issue for users of the Access Control Plugin.

jonbartels commented 3 years ago

A few points:

This bug is interesting in that its an outside-in view of how MC manages JS contexts between channels, but I'm having a hard time seeing it as a bug worth fixing. (My opinion carries exactly ZERO weight around here though! :D )

MichaelLeeHobbs commented 3 years ago

I've not had time to test this and figure out the details but I had this happen.

Code template library A Copied the entire library to B

Major changes to template library in B

In a test channel I switch dependencies from A to B. Worked as expected.

The surprise was the code in B impacted channels that were not redeployed nor had their dependencies changed. When I have time will dig into this.

thorst commented 6 months ago

I came here to report this bug. I will explain below how its affected us and add some things that weren't discussed so far in this bug report.

Context: We initially started coding most code templates to be prototype extensions. Our initial code templates were very generic functions that I wanted added, think lodash/underscore or fleshing out date functionality. We began seeing data cross from one channel to another. We would assign a variable the output value of a code template, and over time we saw that very rarely, it would get data from another channel, another message, that went through at the same time. The simple fix was to change them to regular functions, where I prefix with the object it meant to act on. An example would arrayConcat(), or stringReplace().

New Detail number 1: When you update a code template and redeploy the channel, it doesn't destroy the js engine. So, for example, I created a code template Array.prototype._myFunc2 set the dependency in my channel, deploy it, and run a transaction through. Now I rename the code template to Array.prototype._myFunc3, save the code template, redeploy the channel, run another transaction through and now both _myFunc2 and _myFunc3 will exist. I would have expected _myFunc2 to get destroyed for that channel, since it's no longer defined, but it'll hang out there. If I go remove the dependency from that channel on that code template library, save, deploy my channel again (provided it doesn't call the template), the function STILL exists. This is definitely not expected behavior, because the channel no longer has a dependency on that library. Re-enabling the code template library on that channel, still has the original _myFunc2 and _myFunc3 still.

New detail number 2: This one is a bit weirder. I cannot recreate it. We have a code template that is a prototype extension again, and this one was never enabled as a dependency for the channel, but just like _myFunc2 above, it is visible to the channel. I tried to recreate it, by having other code templates that were prototype extensions, but they only show up as available when you set it to a dependency once. As described above. But this one is visible to all channels. Although I cannot recreate it with another code template, I can show you the code template that I have that's doing it. It persists after mirth is restarted.

Conclusion I agree that this is lower priority, and I've only had issues with this bleeding from one channel to another while using prototype extensions. I would prefer a fix though, because I do like prototype extensions, they allow for cleaner code. For now, the fix is simple, remove all prototype extensions in your code base, however the issue should still be resolved. In the meantime, it should possibly be documented that prototype extensions and mirth don't play nicely together.

thorst commented 6 months ago

Im sure there is an easier way, but we found this out with the following code:

let u = [1, 2, 3];
for (let f in u) {
    echo("index:" + f);
}

It will output:

index:0
index:1
index:2
index:_Radiology_Channel_Map_Searching3
index:_myFunc1
index:_myFunc2

The last three lines are unexpected in the current scenario, as it does not have a dependency on the code template where those functions are defined.

I created a js fiddle example of my code: https://jsfiddle.net/wz0su1d7/