Skip to content

Commit b374f4a

Browse files
bghgaryCopilot
bghgary
and
Copilot
authored
Fix sub-vec4 uniform heap-buffer-overflow (SetInt) (#1637)
## Summary `NativeEngine::SetInt` passed only 1 float to `Program::SetUniform`, but bgfx always reads 16 bytes (vec4) from uniform data via `UniformBuffer::write`. This caused a heap-buffer-overflow detected by AddressSanitizer. ## Changes **Bug fix:** - **NativeEngine.cpp**: `SetInt` now creates a `float[4]` (zero-padded) matching the `SetFloatN` pattern - **Program.h**: Added `bgfx::UniformType::Enum Type` to `UniformInfo` for type-aware validation - **Program.cpp**: Stores `info.type` from `bgfx::getUniformInfo`; adds debug assert in `SetUniform` that verifies data size matches `floatsPerElement * elementLength` (compiles away in release) **Test:** - **Tests.UniformPadding.cpp**: Exercises `setInt`, `setFloat`, `setVector2`, `setVector3` + draw call ## Verification PR #1636 (test-only, without the fix) confirmed both ASAN CI jobs fail: - `MacOS_Sanitizers`: `heap-buffer-overflow` in `bx::memCopy` from `Program::SetUniform` from `SetInt` - `Win32_x64_D3D11_Sanitizers`: same trace --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 3ccba7a commit b374f4a

5 files changed

Lines changed: 127 additions & 4 deletions

File tree

‎Apps/UnitTests/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ set(SOURCES
2020
"Source/Tests.ExternalTexture.cpp"
2121
"Source/Tests.JavaScript.cpp"
2222
"Source/Tests.ShaderCache.cpp"
23+
"Source/Tests.UniformPadding.cpp"
2324
"Source/Utils.h"
2425
"Source/Utils.${GRAPHICS_API}.${BABYLON_NATIVE_PLATFORM_IMPL_EXT}")
2526

‎Apps/UnitTests/Source/Tests.UniformPadding.cpp‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
#include <gtest/gtest.h>
2+
3+
#include <Babylon/AppRuntime.h>
4+
#include <Babylon/Graphics/Device.h>
5+
#include <Babylon/Polyfills/Console.h>
6+
#include <Babylon/Polyfills/Window.h>
7+
#include <Babylon/Plugins/NativeEngine.h>
8+
#include <Babylon/ScriptLoader.h>
9+
10+
#include <future>
11+
#include <iostream>
12+
13+
extern Babylon::Graphics::Configuration g_deviceConfig;
14+
15+
// Exercises all sub-vec4 uniform setter paths followed by a draw call to
16+
// verify that uniform data is padded to at least 16 bytes (vec4).
17+
// bgfx always reads 16 bytes from uniform storage, so anything smaller
18+
// causes a heap-buffer-overflow detectable by AddressSanitizer.
19+
//
20+
// Paths exercised:
21+
// setInt → NativeEngine::SetInt → Program::SetUniform (1 float + 3 pad)
22+
// setFloat → NativeEngine::SetFloat → SetFloatN<1> (1 float + 3 pad)
23+
// setVector2→ NativeEngine::SetFloat2 → SetFloatN<2> (2 floats + 2 pad)
24+
// setVector3→ NativeEngine::SetFloat3 → SetFloatN<3> (3 floats + 1 pad)
25+
TEST(UniformPadding, SubVec4UniformsDoNotOverflow)
26+
{
27+
Babylon::Graphics::Device device{g_deviceConfig};
28+
Babylon::Graphics::DeviceUpdate update{device.GetUpdate("update")};
29+
30+
device.StartRenderingCurrentFrame();
31+
update.Start();
32+
33+
std::promise<void> done{};
34+
35+
Babylon::AppRuntime::Options options{};
36+
options.UnhandledExceptionHandler = [&done](const Napi::Error& error) {
37+
std::cerr << "[Uncaught Error] " << Napi::GetErrorString(error) << std::endl;
38+
done.set_exception(std::make_exception_ptr(std::exception{}));
39+
};
40+
41+
Babylon::AppRuntime runtime{options};
42+
runtime.Dispatch([&device](Napi::Env env) {
43+
device.AddToJavaScript(env);
44+
45+
Babylon::Polyfills::Console::Initialize(env, [](const char* message, auto) {
46+
std::cout << message << std::endl;
47+
});
48+
Babylon::Polyfills::Window::Initialize(env);
49+
Babylon::Plugins::NativeEngine::Initialize(env);
50+
});
51+
52+
Babylon::ScriptLoader loader{runtime};
53+
loader.LoadScript("app:///Assets/babylon.max.js");
54+
55+
// Disable async shader compilation so shaders are ready immediately.
56+
// Create a scene with a ShaderMaterial that declares int, float, vec2,
57+
// and vec3 uniforms. Setting and rendering these exercises every sub-vec4
58+
// code path through Program::SetUniform → DrawInternal → bgfx::setUniform.
59+
loader.Eval(R"(
60+
const engine = new BABYLON.NativeEngine();
61+
engine.getCaps().parallelShaderCompile = null;
62+
const scene = new BABYLON.Scene(engine);
63+
scene.createDefaultCamera();
64+
const sphere = BABYLON.MeshBuilder.CreateSphere("s", { diameter: 1 }, scene);
65+
const mat = new BABYLON.ShaderMaterial("test", scene, {
66+
vertexSource: `
67+
attribute vec3 position;
68+
uniform mat4 worldViewProjection;
69+
void main() { gl_Position = worldViewProjection * vec4(position, 1.0); }
70+
`,
71+
fragmentSource: `
72+
precision highp float;
73+
uniform int testInt;
74+
uniform float testFloat;
75+
uniform vec2 testVec2;
76+
uniform vec3 testVec3;
77+
void main() {
78+
gl_FragColor = vec4(
79+
float(testInt) + testFloat + testVec2.x + testVec3.x,
80+
testVec2.y + testVec3.y,
81+
testVec3.z,
82+
1.0);
83+
}
84+
`
85+
}, {
86+
attributes: ["position"],
87+
uniforms: ["worldViewProjection", "testInt", "testFloat", "testVec2", "testVec3"]
88+
});
89+
mat.setInt("testInt", 1);
90+
mat.setFloat("testFloat", 2.0);
91+
mat.setVector2("testVec2", new BABYLON.Vector2(3.0, 4.0));
92+
mat.setVector3("testVec3", new BABYLON.Vector3(5.0, 6.0, 7.0));
93+
sphere.material = mat;
94+
scene.render();
95+
if (!scene.isReady()) { throw new Error("Scene should be ready with synchronous shader compilation"); }
96+
)", "uniform_padding_test.js");
97+
98+
loader.Dispatch([&done](Napi::Env) {
99+
done.set_value();
100+
});
101+
102+
done.get_future().get();
103+
104+
update.Finish();
105+
device.FinishRenderingCurrentFrame();
106+
}

‎Plugins/NativeEngine/Source/NativeEngine.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1115,8 +1115,8 @@ namespace Babylon
11151115
void NativeEngine::SetInt(NativeDataStream::Reader& data)
11161116
{
11171117
const auto& uniformInfo{*data.ReadPointer<UniformInfo>()};
1118-
const auto value{static_cast<float>(data.ReadInt32())};
1119-
m_currentProgram->SetUniform(uniformInfo.Handle, gsl::make_span(&value, 1));
1118+
const float values[] = {static_cast<float>(data.ReadInt32()), 0.f, 0.f, 0.f};
1119+
m_currentProgram->SetUniform(uniformInfo.Handle, values);
11201120
}
11211121

11221122
template<int size, typename arrayType>

‎Plugins/NativeEngine/Source/Program.cpp‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
#include <arcana/tracing/trace_region.h>
44

5+
#include <cassert>
6+
57
namespace
68
{
79
void InitUniformInfos(
@@ -21,7 +23,7 @@ namespace
2123
bgfx::getUniformInfo(uniforms[index], info);
2224
auto itStage = uniformStages.find(info.name);
2325
auto& handle = uniforms[index];
24-
uniformInfos.emplace(std::make_pair(handle.idx, Babylon::UniformInfo{itStage == uniformStages.end() ? uint8_t{} : itStage->second, handle, info.num}));
26+
uniformInfos.emplace(std::make_pair(handle.idx, Babylon::UniformInfo{itStage == uniformStages.end() ? uint8_t{} : itStage->second, handle, info.type, info.num}));
2527
uniformNameToIndex[info.name] = handleIndex;
2628
}
2729
}
@@ -95,6 +97,18 @@ namespace Babylon
9597
}
9698

9799
value.Data.assign(data.begin(), data.end());
100+
101+
// bgfx reads a type-dependent number of floats per array element from uniform data.
102+
// Must match bgfx g_uniformTypeSize (bgfx.cpp): Vec4=4, Mat3=9, Mat4=16.
103+
assert((itUniformInfo == m_uniformInfos.end() || [&]() {
104+
static_assert(bgfx::UniformType::Vec4 == 2 && bgfx::UniformType::Mat3 == 3 && bgfx::UniformType::Mat4 == 4);
105+
const size_t FloatsPerElement[] = {4, 9, 16};
106+
bgfx::UniformType::Enum type = itUniformInfo->second.Type;
107+
return (type >= bgfx::UniformType::Vec4 && type <= bgfx::UniformType::Mat4)
108+
? value.Data.size() == FloatsPerElement[type - bgfx::UniformType::Vec4] * elementLength
109+
: false;
110+
}()));
111+
98112
value.ElementLength = static_cast<uint16_t>(elementLength);
99113
}
100114

‎Plugins/NativeEngine/Source/Program.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,17 @@ namespace Babylon
1717
{
1818
struct UniformInfo final
1919
{
20-
UniformInfo(uint8_t stage, bgfx::UniformHandle handle, size_t maxElementLength)
20+
UniformInfo(uint8_t stage, bgfx::UniformHandle handle, bgfx::UniformType::Enum type, size_t maxElementLength)
2121
: Stage{stage}
2222
, Handle{handle}
23+
, Type{type}
2324
, MaxElementLength{maxElementLength}
2425
{
2526
}
2627

2728
uint8_t Stage{};
2829
bgfx::UniformHandle Handle{bgfx::kInvalidHandle};
30+
bgfx::UniformType::Enum Type{bgfx::UniformType::Count};
2931
size_t MaxElementLength{};
3032
};
3133

0 commit comments

Comments
 (0)