[SERVER-50917] Unsafe calls to <cctype> functions Created: 14/Sep/20 Updated: 29/Oct/23 Resolved: 13/Oct/20 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Internal Code |
| Affects Version/s: | None |
| Fix Version/s: | 4.9.0 |
| Type: | Improvement | Priority: | Minor - P4 |
| Reporter: | Billy Donahue | Assignee: | Billy Donahue |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | servicearch-wfbf-day | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||
| Sprint: | Service arch 2020-10-19 | ||||||||
| Participants: | |||||||||
| Description |
|
All the <cctype> functions ( isxdigit, isalnum, etc) take an int parameter, not a char. An inspection of our codebase shows that we are passing char to them all the time, and this is incorrect and potentially UB. A negative char will be sign-extended to int, which will be outside the range [0,255]. These functions use table lookups, so this will become an access outside the lookup table bounds. https://en.cppreference.com/w/cpp/header/cctype Typical warning on the cppreference.com docs for such functions.
Another subtle problem with these functions is that all except isdigit and isxdigit are locale-dependent. This is rarely anticipated by callers, who are expecting "C" locale ASCII behavior. We might be better off writing wrappers for these 12 functions and lint-warning against #include <cctype> or #include <ctype.h>. The wrappers can take char, and be locale independent. |
| Comments |
| Comment by Githook User [ 13/Oct/20 ] |
|
Author: {'name': 'Billy Donahue', 'email': 'billy.donahue@mongodb.com', 'username': 'BillyDonahue'}Message: |
| Comment by Githook User [ 13/Oct/20 ] |
|
Author: {'name': 'Billy Donahue', 'email': 'billy.donahue@mongodb.com', 'username': 'BillyDonahue'}Message: |
| Comment by Billy Donahue [ 13/Oct/20 ] |
|
enterprise CR http://mongodbcr.appspot.com/706010001 |
| Comment by Billy Donahue [ 10/Oct/20 ] |