microsoft / BotBuilder-V3

Microsoft Bot Builder V3 SDK for Microsoft Bot Framework
MIT License
51 stars 61 forks source link

Data breach observed when two concurrent send message requests are made to different tenants #245

Closed nagarjun-chodapaneedi closed 2 years ago

nagarjun-chodapaneedi commented 2 years ago

Version

3.11.0

Describe the bug

We have observed a sporadic issue with bot builder when trying to send O365ConnectorCards to two different tenants in Microsoft teams. We tried to send two different O365ConnectorCards to two different sessions but observed that the content is mixed for one of the tenant. Text of one message is replaced with other O365ConnectorCard's text although the title and potentialAction are intact. we are trying to understand if there is a session overlap in botbuilder framework.

To Reproduce

Steps to reproduce the behavior: The issue is sporadic, we have observed this only once. Both the messages were triggered at the same time.

Expected behavior

Content should not be overlapped between two tenant sessions.

Screenshots

Screenshot 2022-03-16 at 10 54 00 AM
anishprasad01 commented 2 years ago

Hi @nagarjun-chodapaneedi,

Something like this might first lead to an examination of state storage. Can you tell us more about how your bot is implemented? Perhaps something in how your bot is accessing and storing state might be causing this?

In your investigations, were you able to reproduce this issue? Were you potentially able to identify which section of the bot's code might be at fault? Could you share some example code relating to the offending functionality that may help us identify what might be causing this?

We need more information before we can examine this.

nagarjun-chodapaneedi commented 2 years ago

Hi @anishprasad01

We are unable to reproduce this issue. This is how we are sending a message.

Building the message using: bot = new builder.UniversalBot(this.chatConnector); // Instance created only once

const session: builder.Session // Created using bot.loadSession for every message send operation
const message: builder.Message // Created using builder.Message(session) for every message send operation

Message is sent using

session.connector.startConversation(address, async (err, response) => {
    if (err) {
        return reject(err);
    }
    session.beginDialog('*:/sendNotificationToTeams', message);
    resolve();
});

Dialog is defined as

bot.dialog('/sendNotificationToTeams',
    (session: builder.Session, message: builder.IMessage, next) => {
        session.send(message).endDialog();
    },
);

We can see the two messages were built in separate instances. we suspect that once we begin a dialog with sendNotificationToTeams there is some overlap of sessions and message objects.

anishprasad01 commented 2 years ago

I will leave this issue open for a couple of days in case the issue reappears, but without a reproducible example, there's nothing we can really do.

diya-biju commented 2 years ago

@anishprasad01 The same issue has happened again when trying to send O365ConnectorCards to two different tenants in Microsoft teams.

Screenshot 2022-04-12 at 8 24 18 PM
anishprasad01 commented 2 years ago

Please provide a specific reproducible example so we can investigate. I also need more information about your tech stack and implementation details related to the issue.

diya-biju commented 2 years ago

@anishprasad01 Not able to reproduce the issue as its sporadic. The implementation is https://github.com/microsoft/BotBuilder-V3/issues/245#issuecomment-1070338799

anishprasad01 commented 2 years ago

Do you have Application Insights or other logs available that may have captured useful information that might help identify the cause?

anishprasad01 commented 2 years ago

@nagarjun-chodapaneedi, @diya-biju,

You could try creating the response message inside the callback. That should help isolate messages from each other.

Additionally, SDK V3 is officially deprecated, and thus official advice would be to upgrade to SDK V4 at your earliest convenience for the latest features and continued support.