Skip to content
Snippets Groups Projects
Forked from Alchemy Viewer / Alchemy Viewer
Source project has a limited visibility.
  • Nat Goodspeed's avatar
    99c040ea
    DRTVWR-575: Fix possible bad indexing in LLSD::operator[](size_t). · 99c040ea
    Nat Goodspeed authored
    One could argue that passing a negative index to an LLSD array should do
    something other than shrug and reference element [0], but as that's legacy
    behavior, it seems all too likely that the viewer sometimes relies on it.
    
    This specific problem arises if the index passed to operator[]() is negative
    -- either with the previous Integer parameter or with size_t (which of course
    reinterprets the negative index as hugely positive). The non-const
    ImplArray::ref() overload checks parameter 'i' and, if it appears negative,
    sets internal 'index' to 0.
    
    But in the next stanza, if (index >= existing size()), it calls resize() to
    scale the internal array up to one more than the requested index. The trouble
    is that it passed resize(i + 1), not the adjusted resize(index + 1).
    
    With a requested index of exactly -1, that would pass resize(0), which would
    result in the ensuing array[0] reference being invalid.
    
    With a requested index less than -1, that would pass resize(hugely positive)
    -- since, whether operator[]() accepts signed LLSD::Integer or size_t,
    resize() accepts std::vector::size_type. Given that the footprint of an LLSD
    array element is at least a pointer, the number of bytes required for
    resize(hugely positive) is likely to exceed available heap storage.
    
    Passing the adjusted resize(index + 1) should defend against that case.
    99c040ea
    History
    DRTVWR-575: Fix possible bad indexing in LLSD::operator[](size_t).
    Nat Goodspeed authored
    One could argue that passing a negative index to an LLSD array should do
    something other than shrug and reference element [0], but as that's legacy
    behavior, it seems all too likely that the viewer sometimes relies on it.
    
    This specific problem arises if the index passed to operator[]() is negative
    -- either with the previous Integer parameter or with size_t (which of course
    reinterprets the negative index as hugely positive). The non-const
    ImplArray::ref() overload checks parameter 'i' and, if it appears negative,
    sets internal 'index' to 0.
    
    But in the next stanza, if (index >= existing size()), it calls resize() to
    scale the internal array up to one more than the requested index. The trouble
    is that it passed resize(i + 1), not the adjusted resize(index + 1).
    
    With a requested index of exactly -1, that would pass resize(0), which would
    result in the ensuing array[0] reference being invalid.
    
    With a requested index less than -1, that would pass resize(hugely positive)
    -- since, whether operator[]() accepts signed LLSD::Integer or size_t,
    resize() accepts std::vector::size_type. Given that the footprint of an LLSD
    array element is at least a pointer, the number of bytes required for
    resize(hugely positive) is likely to exceed available heap storage.
    
    Passing the adjusted resize(index + 1) should defend against that case.
Code owners
Assign users and groups as approvers for specific file changes. Learn more.