Clamp Caption numeric setters to prevent UBSan float->uint UB
The `outline` and `font_size` property setters (and the matching kwargs in __init__) passed a raw float straight into SFML's setOutlineThickness and setCharacterSize. setCharacterSize takes unsigned int, so any negative or out-of-range float produced undefined behavior — UBSan's "value outside the representable range" report. Clamp before the cast. Negative outline is also nonsensical, so it's clamped to 0 for consistency (not UB, but wrong). Fuzz corpus crash tests/fuzz/crashes/property_types-crash-1f7141d732736d04b99d20261abd766194246ea6 now runs cleanly. closes #309 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
328b37d02e
commit
4fd718472a
2 changed files with 79 additions and 3 deletions
|
|
@ -209,11 +209,18 @@ int UICaption::set_float_member(PyUICaptionObject* self, PyObject* value, void*
|
||||||
self->data->markCompositeDirty(); // #289: position change invalidates parent cache
|
self->data->markCompositeDirty(); // #289: position change invalidates parent cache
|
||||||
}
|
}
|
||||||
else if (member_ptr == 4) { //outline
|
else if (member_ptr == 4) { //outline
|
||||||
|
// #309: negative outline thickness is nonsensical; clamp to 0
|
||||||
|
if (val < 0.0f) val = 0.0f;
|
||||||
self->data->text.setOutlineThickness(val);
|
self->data->text.setOutlineThickness(val);
|
||||||
self->data->markDirty(); // #289: content change invalidates own + parent cache
|
self->data->markDirty(); // #289: content change invalidates own + parent cache
|
||||||
}
|
}
|
||||||
else if (member_ptr == 5) { // character size
|
else if (member_ptr == 5) { // character size
|
||||||
self->data->text.setCharacterSize(val);
|
// #309: SFML setCharacterSize takes unsigned int; casting a negative or
|
||||||
|
// out-of-range float is UB. Clamp to a safe range before the cast.
|
||||||
|
// Upper bound is a practical cap well below UINT_MAX to keep SFML sane.
|
||||||
|
if (val < 0.0f) val = 0.0f;
|
||||||
|
else if (val > 65535.0f) val = 65535.0f;
|
||||||
|
self->data->text.setCharacterSize(static_cast<unsigned int>(val));
|
||||||
self->data->markDirty(); // #289: content change invalidates own + parent cache
|
self->data->markDirty(); // #289: content change invalidates own + parent cache
|
||||||
}
|
}
|
||||||
return 0;
|
return 0;
|
||||||
|
|
@ -503,6 +510,8 @@ int UICaption::init(PyUICaptionObject* self, PyObject* args, PyObject* kwds)
|
||||||
self->data = std::make_shared<UICaption>();
|
self->data = std::make_shared<UICaption>();
|
||||||
self->data->position = sf::Vector2f(x, y);
|
self->data->position = sf::Vector2f(x, y);
|
||||||
self->data->text.setPosition(self->data->position);
|
self->data->text.setPosition(self->data->position);
|
||||||
|
// #309: clamp outline to non-negative
|
||||||
|
if (outline < 0.0f) outline = 0.0f;
|
||||||
self->data->text.setOutlineThickness(outline);
|
self->data->text.setOutlineThickness(outline);
|
||||||
|
|
||||||
// Set the font
|
// Set the font
|
||||||
|
|
@ -516,6 +525,9 @@ int UICaption::init(PyUICaptionObject* self, PyObject* args, PyObject* kwds)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Set character size
|
// Set character size
|
||||||
|
// #309: clamp before casting to unsigned int to avoid UB on out-of-range floats
|
||||||
|
if (font_size < 0.0f) font_size = 0.0f;
|
||||||
|
else if (font_size > 65535.0f) font_size = 65535.0f;
|
||||||
self->data->text.setCharacterSize(static_cast<unsigned int>(font_size));
|
self->data->text.setCharacterSize(static_cast<unsigned int>(font_size));
|
||||||
|
|
||||||
// Set text
|
// Set text
|
||||||
|
|
|
||||||
64
tests/regression/issue_309_caption_numeric_setter_test.py
Normal file
64
tests/regression/issue_309_caption_numeric_setter_test.py
Normal file
|
|
@ -0,0 +1,64 @@
|
||||||
|
"""Regression test for issue #309.
|
||||||
|
|
||||||
|
Caption numeric setters (outline, font_size) previously cast raw floats to
|
||||||
|
unsigned int without clamping. Feeding a negative or out-of-range value
|
||||||
|
triggered UBSan under the fuzz harness (fuzz_property_types).
|
||||||
|
|
||||||
|
This test exercises the patched paths with values that would previously be UB:
|
||||||
|
- negative outline
|
||||||
|
- negative font_size
|
||||||
|
- extremely large font_size (beyond UINT_MAX)
|
||||||
|
|
||||||
|
Also exercises the constructor path which had the same bug.
|
||||||
|
"""
|
||||||
|
|
||||||
|
import mcrfpy
|
||||||
|
import sys
|
||||||
|
|
||||||
|
|
||||||
|
def main():
|
||||||
|
# Setter path: create a Caption then poke the values
|
||||||
|
c = mcrfpy.Caption(text="x", pos=(0, 0))
|
||||||
|
|
||||||
|
# Negative outline: used to be UB via setOutlineThickness -> sane now
|
||||||
|
c.outline = -5.0
|
||||||
|
# Post-clamp: the value exposed is the raw float (getter returns float),
|
||||||
|
# but the UB path is gone. Just assert no crash and no Python exception.
|
||||||
|
_ = c.outline
|
||||||
|
|
||||||
|
# Very large outline: float stays a float, not UB, but make sure it doesn't explode
|
||||||
|
c.outline = 1e9
|
||||||
|
_ = c.outline
|
||||||
|
|
||||||
|
# Negative font_size: this was the UBSan trigger (-992.065 in the original crash)
|
||||||
|
c.font_size = -992.065
|
||||||
|
_ = c.font_size # must not crash
|
||||||
|
|
||||||
|
# Font size above UINT_MAX: would cast to huge unsigned, now clamped
|
||||||
|
c.font_size = 1e20
|
||||||
|
_ = c.font_size
|
||||||
|
|
||||||
|
# Constructor path: kwargs with pathological values
|
||||||
|
c2 = mcrfpy.Caption(text="y", pos=(10, 10), outline=-3.0, font_size=-17.5)
|
||||||
|
# Accessing properties must not crash
|
||||||
|
_ = c2.outline
|
||||||
|
_ = c2.font_size
|
||||||
|
|
||||||
|
c3 = mcrfpy.Caption(text="z", pos=(20, 20), font_size=1e15)
|
||||||
|
_ = c3.font_size
|
||||||
|
|
||||||
|
# Type errors should still be raised for non-numeric input
|
||||||
|
try:
|
||||||
|
c.font_size = "not a number"
|
||||||
|
except TypeError:
|
||||||
|
pass
|
||||||
|
else:
|
||||||
|
print("FAIL: font_size should reject strings")
|
||||||
|
sys.exit(1)
|
||||||
|
|
||||||
|
print("PASS")
|
||||||
|
sys.exit(0)
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
main()
|
||||||
Loading…
Add table
Add a link
Reference in a new issue