Skip to content

Commit 74d3c74

Browse files
cast to unsigned char before ctype calls on untrusted input (KhronosGroup#6740)
ctype functions get a plain `char` that can be negative when the byte is >= 0x80, which is undefined for everything but unsigned char and EOF: - text.cpp `spvIsValidIDCharacter` and spirv_target_env.cpp version scan run over raw assembly text - the spec-constant and sampled-image option parsers walk an arbitrary `const char*` - io.cpp hex tokenizer feeds file bytes straight into isspace/tolower/isxdigit cast each argument to unsigned char at the call site. hex_float.h already does the right thing via istream::peek (returns int) so it is left alone.
1 parent a55f5cf commit 74d3c74

5 files changed

Lines changed: 19 additions & 12 deletions

File tree

source/opt/convert_to_sampled_image_pass.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@ using utils::ParseNumber;
3636
// Returns true if the given char is ':', '\0' or considered as blank space
3737
// (i.e.: '\n', '\r', '\v', '\t', '\f' and ' ').
3838
bool IsSeparator(char ch) {
39-
return std::strchr(":\0", ch) || std::isspace(ch) != 0;
39+
return std::strchr(":\0", ch) ||
40+
std::isspace(static_cast<unsigned char>(ch)) != 0;
4041
}
4142

4243
// Reads characters starting from |str| until it meets a separator. Parses a
@@ -436,7 +437,7 @@ ConvertToSampledImagePass::ParseDescriptorSetBindingPairsString(
436437
descriptor_set_binding_pairs->push_back({descriptor_set, binding});
437438

438439
// Skip trailing spaces.
439-
while (std::isspace(*str)) str++;
440+
while (std::isspace(static_cast<unsigned char>(*str))) str++;
440441
}
441442

442443
return descriptor_set_binding_pairs;

source/opt/set_spec_constant_default_value_pass.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,8 @@ Pass::Status SetSpecConstantDefaultValuePass::Process() {
363363
// Returns true if the given char is ':', '\0' or considered as blank space
364364
// (i.e.: '\n', '\r', '\v', '\t', '\f' and ' ').
365365
bool IsSeparator(char ch) {
366-
return std::strchr(":\0", ch) || std::isspace(ch) != 0;
366+
return std::strchr(":\0", ch) ||
367+
std::isspace(static_cast<unsigned char>(ch)) != 0;
367368
}
368369

369370
std::unique_ptr<SetSpecConstantDefaultValuePass::SpecIdToValueStrMap>
@@ -375,7 +376,8 @@ SetSpecConstantDefaultValuePass::ParseDefaultValuesString(const char* str) {
375376
// The parsing loop, break when points to the end.
376377
while (*str) {
377378
// Find the spec id.
378-
while (std::isspace(*str)) str++; // skip leading spaces.
379+
while (std::isspace(static_cast<unsigned char>(*str)))
380+
str++; // skip leading spaces.
379381
const char* entry_begin = str;
380382
while (!IsSeparator(*str)) str++;
381383
const char* entry_end = str;
@@ -407,7 +409,7 @@ SetSpecConstantDefaultValuePass::ParseDefaultValuesString(const char* str) {
407409
(*spec_id_to_value)[spec_id] = std::string(val_begin, val_end - val_begin);
408410

409411
// Skip trailing spaces.
410-
while (std::isspace(*str)) str++;
412+
while (std::isspace(static_cast<unsigned char>(*str))) str++;
411413
}
412414

413415
return spec_id_to_value;

source/spirv_target_env.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,8 @@ bool spvReadEnvironmentFromText(const std::vector<char>& text,
226226
char minor = text[minor_digit_pos];
227227
char next_char =
228228
minor_digit_pos + 1 < text.size() ? text[minor_digit_pos + 1] : 0;
229-
if (std::isdigit(minor) && !std::isdigit(next_char)) {
229+
if (std::isdigit(static_cast<unsigned char>(minor)) &&
230+
!std::isdigit(static_cast<unsigned char>(next_char))) {
230231
const auto index = minor - '0';
231232
assert(index >= 0);
232233
if (static_cast<size_t>(index) < ordered_universal_envs.size()) {
@@ -243,7 +244,7 @@ bool spvReadEnvironmentFromText(const std::vector<char>& text,
243244
for (; i < text.size(); ++i) {
244245
if (text[i] == '\n') break;
245246
}
246-
} else if (!std::isspace(c)) {
247+
} else if (!std::isspace(static_cast<unsigned char>(c))) {
247248
// Allow blanks, but end the search if we find something else.
248249
break;
249250
}

source/text.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@
4545
#include "spirv-tools/libspirv.h"
4646

4747
bool spvIsValidIDCharacter(const char value) {
48-
return value == '_' || 0 != ::isalnum(value);
48+
return value == '_' || 0 != ::isalnum(static_cast<unsigned char>(value));
4949
}
5050

5151
// Returns true if the given string represents a valid ID name.

tools/io.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,9 @@ enum class HexMode {
9696

9797
// Whether a character should be skipped as whitespace / separator /
9898
// end-of-file.
99-
bool IsSpace(char c) { return isspace(c) || c == ',' || c == '\0'; }
99+
bool IsSpace(char c) {
100+
return isspace(static_cast<unsigned char>(c)) || c == ',' || c == '\0';
101+
}
100102

101103
bool IsHexStream(const std::vector<char>& stream) {
102104
for (char c : stream) {
@@ -115,7 +117,8 @@ bool IsHexStream(const std::vector<char>& stream) {
115117

116118
bool MatchIgnoreCase(const char* token, const char* expect, size_t len) {
117119
for (size_t i = 0; i < len; ++i) {
118-
if (tolower(token[i]) != tolower(expect[i])) {
120+
if (tolower(static_cast<unsigned char>(token[i])) !=
121+
tolower(static_cast<unsigned char>(expect[i]))) {
119122
return false;
120123
}
121124
}
@@ -258,7 +261,7 @@ class HexTokenizer {
258261
}
259262

260263
token_chars[i] = c;
261-
if (!isxdigit(c)) {
264+
if (!isxdigit(static_cast<unsigned char>(c))) {
262265
ParseError("encountered non-hex character");
263266
}
264267
}
@@ -272,7 +275,7 @@ class HexTokenizer {
272275
// Consume one hex digit.
273276
char NextHexDigit() {
274277
char c = Next();
275-
if (!isxdigit(c)) {
278+
if (!isxdigit(static_cast<unsigned char>(c))) {
276279
ParseError("encountered non-hex character");
277280
}
278281
return c;

0 commit comments

Comments
 (0)