Game assets are not as passive and as they can seem can be easily overlooked, like in older native engines they are frequently quite compact, engine specific for parsers: key/value blocks, model metadata, collision descriptions, animation tables, material definitions, visibility data and other structured records that eventually cross a trust boundary. If those records are parsed by C or C++ code that assumes a fixed maximum token length but never enforces that maximum, an asset can become memory-corrupting input.

The real issue discussed here was reported in 2017 and patched in shipped Valve games in June 2017.

Scope and source note

One Up Security publicly described the issue on 19 July 2017 as a Source SDK buffer overflow reachable through ragdoll model data. The vulnerable code excerpts below come from Valve’s public source-sdk-2013 repository at commit f56bb35301836e56582a575a75864392a0177875.

The bug class

The bad pattern was not complicated:

  1. A parser accepted a string token from asset-controlled data.
  2. The caller provided a fixed-size stack buffer.
  3. The tokenizer copied bytes into that buffer until a separator or NUL byte.
  4. The tokenizer had no parameter for the destination capacity.
  5. A token longer than the destination buffer caused an out-of-bounds write.

That is the complete bug class. It is also why this kind of issue tends to survive code reviews, the function name looks like a normal parsing utility, the callers buffer looks large enough for ordinary files and the crash only appears when input stops being ordinary.

The relevant Source SDK code

The ragdoll collision-rules parser is implemented as CRagdollCollisionRules and not as a generic ragdoll object method. The relevant historical collisionpair branch looked like this in Valve’s public Source SDK 2013 code:

else if ( !strcmpi( pKey, "collisionpair" ) )
{
    if ( m_bSelfCollisions )
    {
        char szToken[256];
        const char *pStr = nexttoken(szToken, pValue, ',');
        int index0 = atoi(szToken);
        nexttoken( szToken, pStr, ',' );
        int index1 = atoi(szToken);
        m_pSet->EnableCollisions( index0, index1 );
    }
    else
    {
        Assert(0);
    }
}

Two things matter here, first the destination is a stack buffer: char szToken[256]. The parser expects each comma-separated value to fit in 255 bytes plus a terminator.

Secondly the tokenizer call does not include the size of szToken and from the caller’s point of view, there is no way for nexttoken to know how much memory it is allowed to write.

The historical nexttoken implementation confirms that problem:

const char *nexttoken(char *token, const char *str, char sep)
{
    if ((str == NULL) || (*str == '\0'))
    {
        *token = '\0';
        return(NULL);
    }

    while ((*str != sep) && (*str != '\0'))
    {
        *token++ = *str++;
    }

    *token = '\0';

    if (*str == '\0')
    {
        return(str);
    }

    return(++str);
}

There is no length check in the copy loop and the loop terminates on sep or \0, not on available destination capacity. So, for a caller that passes char szToken[256], any token longer than the buffer is outside the parsers contract even though the function does not encode that contract in its API.

This is technically the core failure: the API shape made safe use impossible to enforce. A C function that writes to caller-provided memory must either receive the destination length, use a type that carries length, or otherwise prove that the source length is bounded before copying.

What a real fix has to change

A real fix cannot just say, “make the buffer bigger.” because that changes the crash threshold but not the bug. The fix needs to change the parser contract so the tokenizer knows the size of the destination buffer and consistently handles overlong tokens.

A public Source SDK-derived implementation, Mapbase, shows the bounded shape of this fix by adding size_t tokenLen to nexttoken:

const char *nexttoken(char *token, const char *str, char sep, size_t tokenLen)
{
    if ((str == NULL) || (*str == '\0'))
    {
        if(tokenLen)
        {
            *token = '\0';
        }
        return(NULL);
    }

    while ((*str != sep) && (*str != '\0') && (tokenLen > 1))
    {
        *token++ = *str++;
        tokenLen--;
    }

    // If token is to big for return buffer, skip rest of token.
    while ((*str != sep) && (*str != '\0'))
    {
        str++;
    }

    if(tokenLen)
    {
        *token = '\0';
        tokenLen--;
    }

    if (*str == '\0')
    {
        return(str);
    }

    return(++str);
}

That is the right general direction, the destination capacity becomes part of the function contract, the copy loop reserves space for a terminator and the parser consumes the rest of an overlong token instead of leaving the input stream half-parsed. In a production codebase the declaration, implementation and every call site should all need to agree on the new contract.

For new C++ code a cleaner design is usually to avoid raw destination buffers entirely. Returning a std::string_view token over the original input, then validating and converting it, avoids copying until a bounded copy is explicitly required.

Safe reconstruction: a toy parser, not the Source parser

The rest of this post uses a tiny toy format to reproduce the bug class without reproducing the Source exploit. It is intentionally not compatible with Source model files.

Toy record format:

PAIR <integer>,<integer>\n

Examples:

PAIR 1,2
PAIR 12,44
PAIR 999,1000

The parser will read the two comma-separated tokens and convert them to integer indices. The vulnerable version will make the same category of mistake as the historical bug: copying an unbounded token into a fixed stack buffer.

Minimal project layout

ragdoll-parser-lab/
├── CMakeLists.txt
└── src/
    ├── unsafe_parser.cpp
    ├── safe_parser.cpp
    └── fuzz_safe_parser.cpp

Vulnerable toy parser

This file is deliberately unsafe. It exists to make the bug class obvious under AddressSanitizer.

// src/unsafe_parser.cpp
#include <cctype>
#include <cstdlib>
#include <cstring>
#include <iostream>
#include <stdexcept>
#include <string>

static const char* next_token_unsafe(char* token, const char* str, char sep) {
    if (str == nullptr || *str == '\0') {
        *token = '\0';
        return nullptr;
    }

    while (*str != sep && *str != '\0') {
        *token++ = *str++;
    }

    *token = '\0';

    if (*str == '\0') {
        return str;
    }

    return str + 1;
}

struct CollisionPair {
    int first;
    int second;
};

static CollisionPair parse_pair_unsafe(const std::string& line) {
    constexpr const char* prefix = "PAIR ";

    if (line.rfind(prefix, 0) != 0) {
        throw std::runtime_error("expected PAIR record");
    }

    const char* input = line.c_str() + std::strlen(prefix);
    char token[256];

    const char* rest = next_token_unsafe(token, input, ',');
    int first = std::atoi(token);

    next_token_unsafe(token, rest, '\n');
    int second = std::atoi(token);

    return {first, second};
}

int main(int argc, char** argv) {
    if (argc != 2) {
        std::cerr << "usage: unsafe_parser 'PAIR 1,2'\n";
        return 2;
    }

    try {
        CollisionPair pair = parse_pair_unsafe(argv[1]);
        std::cout << "first=" << pair.first << " second=" << pair.second << "\n";
        return 0;
    } catch (const std::exception& e) {
        std::cerr << "parse error: " << e.what() << "\n";
        return 1;
    }
}

The problem is local and mechanical: next_token_unsafe accepts char* token, but it has no idea how large token is. The caller passes char token[256]; the callee copies until a comma or NUL byte. Those two facts are incompatible when the input is attacker-controlled or even just malformed.

Build it with sanitizers:

cmake -S . -B build -DCMAKE_BUILD_TYPE=Debug
cmake --build build

A long first token should produce a stack-buffer-overflow report under ASan:

./build/unsafe_parser "PAIR $(python3 - <<'PY'
print('9' * 300)
PY
),1"

The exact ASan output depends on compiler and platform, but the important part is the write past the end of the local token buffer in next_token_unsafe.

Safe toy parser

The safe version avoids copying into a fixed buffer. It tokenizes with std::string_view, validates each token and only then converts it to an integer.

// src/safe_parser.cpp
#include <charconv>
#include <cctype>
#include <iostream>
#include <limits>
#include <stdexcept>
#include <string>
#include <string_view>

struct CollisionPair {
    int first;
    int second;
};

static std::string_view trim(std::string_view value) {
    while (!value.empty() && std::isspace(static_cast<unsigned char>(value.front()))) {
        value.remove_prefix(1);
    }

    while (!value.empty() && std::isspace(static_cast<unsigned char>(value.back()))) {
        value.remove_suffix(1);
    }

    return value;
}

static int parse_int_strict(std::string_view token) {
    token = trim(token);

    if (token.empty()) {
        throw std::runtime_error("empty integer token");
    }

    int result = 0;
    const char* begin = token.data();
    const char* end = token.data() + token.size();
    auto [ptr, ec] = std::from_chars(begin, end, result);

    if (ec != std::errc{} || ptr != end) {
        throw std::runtime_error("invalid integer token");
    }

    if (result < 0) {
        throw std::runtime_error("negative collision index");
    }

    return result;
}

static CollisionPair parse_pair_safe(std::string_view line) {
    constexpr std::string_view prefix = "PAIR ";

    if (!line.starts_with(prefix)) {
        throw std::runtime_error("expected PAIR record");
    }

    line.remove_prefix(prefix.size());

    const std::size_t comma = line.find(',');
    if (comma == std::string_view::npos) {
        throw std::runtime_error("missing comma");
    }

    std::string_view first_token = line.substr(0, comma);
    std::string_view second_token = line.substr(comma + 1);

    return {
        parse_int_strict(first_token),
        parse_int_strict(second_token)
    };
}

int main(int argc, char** argv) {
    if (argc != 2) {
        std::cerr << "usage: safe_parser 'PAIR 1,2'\n";
        return 2;
    }

    try {
        CollisionPair pair = parse_pair_safe(argv[1]);
        std::cout << "first=" << pair.first << " second=" << pair.second << "\n";
        return 0;
    } catch (const std::exception& e) {
        std::cerr << "parse error: " << e.what() << "\n";
        return 1;
    }
}

This version fixes more than the overflow:

  • It does not copy token bytes into a stack buffer.
  • It rejects empty tokens.
  • It rejects partial parses such as 12abc.
  • It rejects negative indices.
  • It keeps tokenization separate from integer conversion.

The important shift is that malformed input becomes a parser error, not memory corruption.

Simple grammar-aware fuzzer

A byte-level fuzzer is useful, but a small grammar-aware mutator quickly reaches the parser states we care about. This fuzzer generates both ordinary records and pathological records with long tokens.

// src/fuzz_safe_parser.cpp
#include <charconv>
#include <cctype>
#include <cstdint>
#include <iostream>
#include <random>
#include <stdexcept>
#include <string>
#include <string_view>
#include <vector>

struct CollisionPair {
    int first;
    int second;
};

static std::string_view trim(std::string_view value) {
    while (!value.empty() && std::isspace(static_cast<unsigned char>(value.front()))) {
        value.remove_prefix(1);
    }

    while (!value.empty() && std::isspace(static_cast<unsigned char>(value.back()))) {
        value.remove_suffix(1);
    }

    return value;
}

static int parse_int_strict(std::string_view token) {
    token = trim(token);

    if (token.empty()) {
        throw std::runtime_error("empty integer token");
    }

    int result = 0;
    const char* begin = token.data();
    const char* end = token.data() + token.size();
    auto [ptr, ec] = std::from_chars(begin, end, result);

    if (ec != std::errc{} || ptr != end) {
        throw std::runtime_error("invalid integer token");
    }

    if (result < 0) {
        throw std::runtime_error("negative collision index");
    }

    return result;
}

static CollisionPair parse_pair_safe(std::string_view line) {
    constexpr std::string_view prefix = "PAIR ";

    if (!line.starts_with(prefix)) {
        throw std::runtime_error("expected PAIR record");
    }

    line.remove_prefix(prefix.size());

    const std::size_t comma = line.find(',');
    if (comma == std::string_view::npos) {
        throw std::runtime_error("missing comma");
    }

    std::string_view first_token = line.substr(0, comma);
    std::string_view second_token = line.substr(comma + 1);

    return {
        parse_int_strict(first_token),
        parse_int_strict(second_token)
    };
}

static std::string random_digits(std::mt19937& rng, std::size_t max_len) {
    std::uniform_int_distribution<std::size_t> len_dist(0, max_len);
    std::uniform_int_distribution<int> digit_dist(0, 9);

    const std::size_t len = len_dist(rng);
    std::string out;
    out.reserve(len);

    for (std::size_t i = 0; i < len; ++i) {
        out.push_back(static_cast<char>('0' + digit_dist(rng)));
    }

    return out;
}

static std::string make_case(std::mt19937& rng) {
    std::uniform_int_distribution<int> mode_dist(0, 7);
    const int mode = mode_dist(rng);

    switch (mode) {
        case 0:
            return "PAIR " + random_digits(rng, 8) + "," + random_digits(rng, 8);
        case 1:
            return "PAIR " + random_digits(rng, 1024) + ",1";
        case 2:
            return "PAIR 1," + random_digits(rng, 1024);
        case 3:
            return "PAIR ," + random_digits(rng, 8);
        case 4:
            return "PAIR " + random_digits(rng, 8);
        case 5:
            return "PAIR -1,2";
        case 6:
            return "NOPE 1,2";
        default:
            return "PAIR 12abc,34";
    }
}

int main() {
    std::mt19937 rng(0xC0111510);

    std::size_t accepted = 0;
    std::size_t rejected = 0;

    for (std::size_t i = 0; i < 1'000'000; ++i) {
        const std::string input = make_case(rng);

        try {
            (void)parse_pair_safe(input);
            ++accepted;
        } catch (const std::exception&) {
            ++rejected;
        }
    }

    std::cout << "accepted=" << accepted << " rejected=" << rejected << "\n";
}

The expected result is not that every generated input parses. The expected result is that malformed input is rejected without crashing, corrupting memory, or producing partial conversions.

CMake build

cmake_minimum_required(VERSION 3.20)
project(ragdoll_parser_lab LANGUAGES CXX)

set(CMAKE_CXX_STANDARD 20)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(CMAKE_CXX_EXTENSIONS OFF)

function(enable_hardening target)
    if (CMAKE_CXX_COMPILER_ID MATCHES "Clang|GNU")
        target_compile_options(${target} PRIVATE
            -Wall -Wextra -Wpedantic
            -fsanitize=address,undefined
            -fno-omit-frame-pointer
        )
        target_link_options(${target} PRIVATE
            -fsanitize=address,undefined
        )
    endif()
endfunction()

add_executable(unsafe_parser src/unsafe_parser.cpp)
enable_hardening(unsafe_parser)

add_executable(safe_parser src/safe_parser.cpp)
enable_hardening(safe_parser)

add_executable(fuzz_safe_parser src/fuzz_safe_parser.cpp)
enable_hardening(fuzz_safe_parser)

What to audit in old asset parsers

This historical issue is a useful checklist item for old game code, file importers, mod tools and native asset pipelines.

Look for functions with signatures like these:

char* parse(char* out, const char* in);
void split(char* out, const char* in, char sep);
const char* next_token(char* out, const char* in, char sep);

The risk is not the names. The risk is an output pointer with no output size. When the input can be influenced by a file, network content, downloaded content, user-generated content, or a mod package, that is a bug-shaped API.

Prefer APIs with explicit size or non-copying views:

bool next_token(char* out, std::size_t out_len, const char* in, char sep);
std::optional<std::string_view> next_token(std::string_view& input, char sep);

Then verify the behavior on overlong input. Safe behavior should be boring: reject the record, truncate only when truncation is explicitly acceptable, preserve parser synchronization and never write outside the destination object.

Why this still matters

The 2017 ragdoll issue is old, but the underlying mistake is still common. Asset parsers often sit at the boundary between high-trust engine code and low-trust content. They also tend to be old, performance-oriented, hand-written and lightly tested against malformed inputs.

The security lesson is narrower and more durable than the original exploit:

  • Asset files are parser input, not trusted data.
  • Stack buffers do not become safe because ordinary assets are short.
  • A function that writes into caller memory needs the caller memory’s size.
  • Parser errors should stay parser errors; they should not become memory corruption.
  • Fuzzing should include grammar-aware cases that reach semantic parser paths, not only random bytes.

The serious fix is not to blacklist a specific asset or expand a specific buffer. The serious fix is to make the parser contract enforceable.

References