Repository navigation
Bound the C API's error message to its buffer, and enlarge the buffer - #16
Merged
Merged
Conversation
Every C API error copied its message into tiledb_errmsg, a 2000-byte global, with strcpy. Messages name arrays and files, whose paths can be up to TILEDB_NAME_MAX_LEN (4096) characters, so a long enough name overran the buffer: a missing array with a 3,000-character name crashed the process. Errors are now copied with snprintf through set_tiledb_errmsg(), truncating if needed, and the buffer is 8192 bytes, room for a message naming two paths of the maximum length.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As @nalinigans pointed out on #8: every C API error copies its message into
tiledb_errmsg, a 2000-byte global, withstrcpy. Messages name arrays and files, whose paths can be up toTILEDB_NAME_MAX_LEN(4096) characters, so a long enough name overruns the buffer: for a missing array, a name of about 1,950 characters overruns it silently, and one of 3,000 characters crashed the process. All 77 copies now go through a newset_tiledb_errmsg(), which usessnprintfand truncates if needed, and the buffer is 8192 bytes, room for a message naming two paths of the maximum length. The copies that already usedsnprintfwere bounded and are unchanged.New tests check that the error for a missing array with a 4,000-character name holds the whole name, which overruns the buffer on
main, and that a message longer than the buffer is truncated to fit.