Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential array out of bounds access in ParticleEffect2D #993

Closed
samoatesgames opened this issue Sep 2, 2016 · 2 comments
Closed

Potential array out of bounds access in ParticleEffect2D #993

samoatesgames opened this issue Sep 2, 2016 · 2 comments

Comments

@samoatesgames
Copy link

In ParticleEffect2D line 187, access is made to srcBlendFuncs and destBlendFuncs using the an index of 0 -> MAX_BLENDMODES.

MAX_BLENDMODES is 9, srcBlendFuncs and destBlendFuncs are arrays with 7 elements.
So potentially array element 8 could be accessed.

It looks like the srcBlendFuncs and destBlendFuncs are missing an entry, however I'm not sure what the value should be, else i'd make a PR.

Code in question:

static const int srcBlendFuncs[] =
{
    1,      // GL_ONE
    1,      // GL_ONE
    0x0306, // GL_DST_COLOR
    0x0302, // GL_SRC_ALPHA
    0x0302, // GL_SRC_ALPHA
    1,      // GL_ONE
    0x0305  // GL_ONE_MINUS_DST_ALPHA
};

static const int destBlendFuncs[] =
{
    0,      // GL_ZERO
    1,      // GL_ONE
    0,      // GL_ZERO
    0x0303, // GL_ONE_MINUS_SRC_ALPHA
    1,      // GL_ONE
    0x0303, // GL_ONE_MINUS_SRC_ALPHA
    0x0304  // GL_DST_ALPHA
};

enum BlendMode
{
    BLEND_REPLACE = 0,
    BLEND_ADD,
    BLEND_MULTIPLY,
    BLEND_ALPHA,
    BLEND_ADDALPHA,
    BLEND_PREMULALPHA,
    BLEND_INVDESTALPHA,
    BLEND_SUBTRACT,
    BLEND_SUBTRACTALPHA,
    MAX_BLENDMODES
};

for (int i = 0; i < MAX_BLENDMODES; ++i)
{
    if (blendFuncSource == srcBlendFuncs[i] && blendFuncDestination == destBlendFuncs[i])
    {
        blendMode_ = (BlendMode)i;
        break;
    }
}

Sam.

@JoshEngebretson JoshEngebretson added this to the Atomic Build 2 milestone Nov 14, 2016
@ghost ghost self-assigned this Dec 5, 2016
@ghost
Copy link

ghost commented Dec 5, 2016

This is an oversight from Urho3D, Its been there since version 1.4.x, so probably forever. The last two modes, BLEND_SUBTRACT and BLEND_SUBTRACTALPHA are only used when there is a light and the light is negative. Particle2D does not support these functions, so it's just possible crash code. I filled out the two blend function arrays with BLEND_ADD and BLEND_ADDALPHA values, so it will not crash if those enums are programmed, and this models how the enums are used when lights are present.

ParticleEmitter2D does not respond correctly to the duration parameter from pex file, and this too is a Urho3D oversight. The line "if (emissionTime_ >= 0.0f)", the comparison should be ">" instead of ">=", otherwise the particle emits forever, instead of stopping when a duration is set.
When a pex file does not include the duration parameter, it defaults to a looping particle. You can change pex file with a duration parameter to looping, by assigning it to -1.0 To test this in a sample that uses a pex file, add the line "<duration value="3.00"/>" somewhere after the <particleEmitterConfig> tag. and that particle will only emit for 3 seconds.

JoshEngebretson added a commit that referenced this issue Dec 5, 2016
Potential array out of bounds access #993
@JoshEngebretson
Copy link
Contributor

Closed via #1243

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants