Azure / durabletask

Durable Task Framework allows users to write long running persistent workflows in C# using the async/await capabilities.
Apache License 2.0
1.47k stars 287 forks source link

[Proposal] Detect and fail fast for dangerous to deserialize types in task activity and task orchestration. #1070

Open pasaini-microsoft opened 2 months ago

pasaini-microsoft commented 2 months ago

Motivation

Regarding issue: #903 Stuck orchestrations at random on control-queue The investigation surfaced that in task-activities via interface were calling method with cancellation token in their parameters. Since, it becomes scheduling and running a task-activity with serialization and deserialization of parameters and return types, cancellation token was also getting deserialized. Deserializing a cancellation token, or any object with safehandle, is dangerous as it can corrupt the native objects and result in undesirable outcomes. E.g., for us, semaphore's locking handle got corrupt and both waitasync and release got stuck making DTF not to proceed.

Since DTF TaskOrchestration and TaskActivities do serialize and deserialize its input(parameters) and output(return type), we had fixed this in our local repo by adding guards around such task-activities and orchestration to avoid objects having safehandle as parameters or return types. Post this check, we are not hitting this issue anymore.

Since, DTF's inherent nature is such it has to serialize and deserialize input and output of task-activity and task-orchestration, it becomes important that DTF upfront understands such dangerous usage and throw to restrict bad practices. This was the motivation behind making this change in DTF core library.

Proposal

Proposal is to add an extension to validate any type for dangerous deserialization and use it to check task-orchestration and task-activity's input and output.