Skip to content

Commit 7bec489

Browse files
committed
Fixes to ShadingSystemImpl::decode_connected_param
@marsupial correctly points out that we are unsafely passing a string_view.data() into both atoi and strchr, which is unsafe because there's no guarantee that a string_view is null-terminated. Ugh! He proposed a fix here: #800 and there's nothing wrong with it per se, but I suspected it could be done more simply, so this PR is my stab at it, using the `OIIO::Strutil::parse_*` functions.
1 parent 0c6e557 commit 7bec489

1 file changed

Lines changed: 26 additions & 11 deletions

File tree

src/liboslexec/shadingsys.cpp

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2482,10 +2482,9 @@ ShadingSystemImpl::decode_connected_param (string_view connectionname,
24822482

24832483
// Look for a bracket in the "parameter name"
24842484
size_t bracketpos = connectionname.find ('[');
2485-
const char *bracket = bracketpos == string_view::npos ? NULL
2486-
: connectionname.data()+bracketpos;
24872485
// Grab just the part of the param name up to the bracket
24882486
ustring param (connectionname, 0, bracketpos);
2487+
string_view cname_remaining = connectionname.substr (bracketpos);
24892488

24902489
// Search for the param with that name, fail if not found
24912490
c.param = inst->findsymbol (param);
@@ -2514,36 +2513,52 @@ ShadingSystemImpl::decode_connected_param (string_view connectionname,
25142513

25152514
c.type = sym->typespec();
25162515

2517-
if (bracket && c.type.is_array()) {
2516+
if (! cname_remaining.empty() && c.type.is_array()) {
25182517
// There was at least one set of brackets that appears to be
25192518
// selecting an array element.
2520-
c.arrayindex = atoi (bracket+1);
2519+
int index = 0;
2520+
if (! (Strutil::parse_char (cname_remaining, '[') &&
2521+
Strutil::parse_int (cname_remaining, index) &&
2522+
Strutil::parse_char (cname_remaining, ']'))) {
2523+
error ("ConnectShaders: malformed parameter \"%s\"", connectionname);
2524+
c.param = -1; // mark as invalid
2525+
return c;
2526+
}
2527+
c.arrayindex = index;
25212528
if (c.arrayindex >= c.type.arraylength()) {
25222529
error ("ConnectShaders: cannot request array element %s from a %s",
25232530
connectionname, c.type.c_str());
25242531
c.arrayindex = c.type.arraylength() - 1; // clamp it
25252532
}
25262533
c.type.make_array (0); // chop to the element type
2527-
bracket = strchr (bracket+1, '['); // skip to next bracket
2534+
Strutil::skip_whitespace (cname_remaining); // skip to next bracket
25282535
}
25292536

2530-
if (bracket && ! c.type.is_closure() &&
2531-
c.type.aggregate() != TypeDesc::SCALAR) {
2537+
if (! cname_remaining.empty() && cname_remaining.front() == '[' &&
2538+
! c.type.is_closure() && c.type.aggregate() != TypeDesc::SCALAR) {
25322539
// There was at least one set of brackets that appears to be
25332540
// selecting a color/vector component.
2534-
c.channel = atoi (bracket+1);
2541+
int index = 0;
2542+
if (! (Strutil::parse_char (cname_remaining, '[') &&
2543+
Strutil::parse_int (cname_remaining, index) &&
2544+
Strutil::parse_char (cname_remaining, ']'))) {
2545+
error ("ConnectShaders: malformed parameter \"%s\"", connectionname);
2546+
c.param = -1; // mark as invalid
2547+
return c;
2548+
}
2549+
c.channel = index;
25352550
if (c.channel >= (int)c.type.aggregate()) {
25362551
error ("ConnectShaders: cannot request component %s from a %s",
25372552
connectionname, c.type.c_str());
25382553
c.channel = (int)c.type.aggregate() - 1; // clamp it
25392554
}
25402555
// chop to just the scalar part
25412556
c.type = TypeSpec ((TypeDesc::BASETYPE)c.type.simpletype().basetype);
2542-
bracket = strchr (bracket+1, '['); // skip to next bracket
2557+
Strutil::skip_whitespace (cname_remaining);
25432558
}
25442559

2545-
// Deal with left over brackets
2546-
if (bracket) {
2560+
// Deal with left over nonsense or unsupported param designations
2561+
if (! cname_remaining.empty()) {
25472562
// Still a leftover bracket, no idea what to do about that
25482563
error ("ConnectShaders: don't know how to connect '%s' when \"%s\" is a \"%s\"",
25492564
connectionname, param.c_str(), c.type.c_str());

0 commit comments

Comments
 (0)