Buffer Overflow and Insufficient Session Expiration

[ramvisa]

sonatype-2023-1010 The github.com/microsoft/go-mssqldb package is vulnerable to Buffer Overflow attacks. The readPLPType() function in the types.go file uses the size defined by an RPC message for the read buffer instead of a fixed buffer size when handling PLP types. An attacker can exploit this behavior by supplying a specially-crafted message that would cause a large memory allocation leading to memory corruption, an application crash or other unexpected behavior.

sonatype-2021-4899 The gorilla/sessions package is vulnerable due to Insufficient Session Expiration. The library allows for the creation of session cookies with the NewCookieStore() function in store.go. However, there is no mechanism available for invalidating user sessions once they have been created in this way. The documentation instructs users to set the MaxAge attribute of a cookie to -1 using the MaxAge() function in order to invalidate the session associated with it. However, this does not invalidate the users session on the server. A malicious user who is able to retrieve the value of a users' session cookie through a Cross-Site Scripting (XSS) attack, a Man-in-the-Middle (MitM) attack, or by some other means, will be able to use that session cookie to impersonate the user even after that user has logged out.

[shueybubbles]

thx for opening an issue!
Why is gorilla/sessions relevant here?

[ramvisa]

There is an indirect dependency on gorilla/sessions. Please see: https://github.com/microsoft/go-mssqldb/blob/main/go.sum#L37
This dependency is being flagged by Sonatype as a finding.

[ramvisa]

The readPLPType method is here:

go-mssqldb/types.go

Line 673 in 9b84d9b

func readPLPType(ti *typeInfo, r *tdsBuffer, c *cryptoMetadata) interface{} {

[shueybubbles]

ok. We can probably address the PLP issue but I don't know what we'd do about gorilla unless there's a patched version now available to whatever dependency is bringing it in.
The mitigation for the PLP issue would be to have a valid certificate on your SQL server and only use encrypted connections. For Azure SQL DB and SQL 2022 it'd be best to use Strict encryption to avoid any MITM.

[shueybubbles]

It's not clear to me that this is a severe problem.
The code has to scan the chunks until it reads a chunk size of 0 regardless of what size was passed in the header. If the size in the header doesn't match the total size of all the chunks, it's essentially the same as the _UNKNOWN_PLP_LEN case where 1000 is just an initial capacity of the buffer and the actual data may end up larger or smaller.

Even if we were to always start with a smaller capacity, if the actual data size is very large (up to 2GB), growing the buffer could fail at some point. What's the difference between that kind of failure vs a failure to allocate that much capacity initially? Failing up front would be faster than failing after however many buffer reallocations and copies to grow it.