rebus-org / Rebus.SqlServer

:bus: Microsoft SQL Server transport and persistence for Rebus
https://mookid.dk/category/rebus
Other
43 stars 42 forks source link

Untrusted Initialization Issue with ConnectionStrings #33

Closed komainu85 closed 5 years ago

komainu85 commented 5 years ago

We have been running some security tests on our project and the 3rd party package we consume.

A security issue has been flagged on Rebus.SqlServer. The issue boils down to the extension methods that take parameter connectionStringOrConnectionStringName. As any connection string can be passed into the method (untrusted data), rather than just taking in a connection string name which only allows connection strings defined within the app.config.

CWE ID 15 This call to system_data_sqlclient_dll.System.Data.SqlClient.SqlConnection.!newinit_0_1() allows external control of system settings. The argument to the function is constructed using untrusted input, which can disrupt service or cause an application to behave in unexpected ways. The first argument to !newinit_0_1() contains tainted data. The tainted data originated from earlier calls to rebus_sqlserver_dll.rebus.config.sqlservertransportconfigurationextensions.usesqlserver, rebus_sqlserver_dll.rebus.config.sqlservertransportconfigurationextensions.usesqlserverasonewayclient, rebus_sqlserver_dll.rebus.config.sqlservertimeoutsconfigurationextensions.storeinsqlserver, rebus_sqlserver_dll.rebus.config.sqlserversubscriptionsconfigurationextensions.storeinsqlserver, rebus_sqlserver_dll.rebus.config.sqlserversagasnapshotsconfigurationextensions.storeinsqlserver, rebus_sqlserver_dll.rebus.config.sqlserversagaconfigurationextensions.storeinsqlserver, and rebus_sqlserver_dll.rebus.config.sqlserverdatabusconfigurationextensions.storeinsqlserver.

mookid8000 commented 5 years ago

øøøh what?

Sorry, but I fail to understand why passing in a connection string from C# is less safe that picking it up from a piece of XML in the file system.

You could even argue that the piece of XML in the file system was easier to tamper with after the fact.

Please tell me more if there's something I'm missing here 🤔