Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
farpeter98 opened this issue Nov 7, 2024 · 3 comments

Comments

@farpeter98
Copy link

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:

auto c_msg = swcvec2str(msg, msg_len);

The data was initialized just a few lines earlier:

while ((rc2 = SQLGetDiagRec(HandleType, handle, i, sql_state.data(), &native_error, msg.data(), msg.capacity(), &msg_len)) != SQL_NO_DATA) {

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
Copy link
Owner

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

@TimelordUK
Copy link
Owner

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants