Read access violation caused by large printed messages in stored procedure invocations

[farpeter98]

using msnodesqlv8 v4.2.1 with sqlserver2022

When a print statement is executed as part of a stored procedure invocation, if the print statement outputs a string larger than the hardcoded 2048 wchars data is read from past the end of a vector.

The issue is caused by the following line: https://github.com/TimelordUK/node-sqlserver-v8/blob/6e9af43f313b796199c8279dacf86031f3a05667/src/OdbcHandle.cpp#L116

The data was initialized just a few lines earlier: https://github.com/TimelordUK/node-sqlserver-v8/blob/6e9af43f313b796199c8279dacf86031f3a05667/src/OdbcHandle.cpp#L111

SQLGetDiagRec returns the length of the whole message length in msg_len and properly truncates the data if necessary but the swcvec2str invocation doesn't check if msg.capacity() < msg_len resulting in the swcvec2str implementation reading from past the end of msg.

Other odbc functions are similarly vulnerable, e.g. in the same OdbcHandle::read_errors function serverName and procName also use 128 byte buffers without checking if the actual values are larger.

The workaround was simply removing the print statements from the stored procedures since they were only left there for debugging.

[TimelordUK]

thansk a lot for raising this - will try and fix this to truncate correctly

[TimelordUK]

i had a go at fixing this on master, need to add tests and release, appreciate raising this - amazing how after all these years these little gremlins keep coming.

[farpeter98]

Heh, thanks, I'll admit initially I thought this might be a serious security concern but after thinking it through a bit more (I'm not a security expert), as a read from a heap memory block allocated by the odbc driver it shouldn't be a real risk (?), although the crash is ugly, but I'd be curious to hear your opinion on the matter, if you have any.

[TimelordUK]

this should be resolved now, im closing it.