Merge pull request #4463 from jakrams/user/jakras/bitangent-fuzzer-fix

Prevent nullptr access to normals-array in bitangent computation
pull/4440/head^2
Kim Kulling 2022-04-12 11:10:47 +02:00 committed by GitHub
commit dc8ba61095
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 23 additions and 20 deletions

View File

@ -284,7 +284,7 @@ static aiMaterial *ImportMaterial(std::vector<int> &embeddedTexIdxs, Asset &r, M
aimat->AddProperty(&alphaMode, AI_MATKEY_GLTF_ALPHAMODE); aimat->AddProperty(&alphaMode, AI_MATKEY_GLTF_ALPHAMODE);
aimat->AddProperty(&mat.alphaCutoff, 1, AI_MATKEY_GLTF_ALPHACUTOFF); aimat->AddProperty(&mat.alphaCutoff, 1, AI_MATKEY_GLTF_ALPHACUTOFF);
//pbrSpecularGlossiness // pbrSpecularGlossiness
if (mat.pbrSpecularGlossiness.isPresent) { if (mat.pbrSpecularGlossiness.isPresent) {
PbrSpecularGlossiness &pbrSG = mat.pbrSpecularGlossiness.value; PbrSpecularGlossiness &pbrSG = mat.pbrSpecularGlossiness.value;
@ -606,7 +606,10 @@ void glTF2Importer::ImportMeshes(glTF2::Asset &r) {
} }
} }
if (needTangents) { if (needTangents) {
if (target.tangent[0]->count != aim->mNumVertices) { if (!aiAnimMesh.HasNormals()) {
// prevent nullptr access to aiAnimMesh.mNormals below when no normals are available
ASSIMP_LOG_WARN("Bitangents of target ", i, " in mesh \"", mesh.name, "\" can't be computed, because mesh has no normals.");
} else if (target.tangent[0]->count != aim->mNumVertices) {
ASSIMP_LOG_WARN("Tangents of target ", i, " in mesh \"", mesh.name, "\" does not match the vertex count"); ASSIMP_LOG_WARN("Tangents of target ", i, " in mesh \"", mesh.name, "\" does not match the vertex count");
} else { } else {
Tangent *tangent = nullptr; Tangent *tangent = nullptr;
@ -698,12 +701,12 @@ void glTF2Importer::ImportMeshes(glTF2::Asset &r) {
nFaces = count - 2; nFaces = count - 2;
facePtr = faces = new aiFace[nFaces]; facePtr = faces = new aiFace[nFaces];
for (unsigned int i = 0; i < nFaces; ++i) { for (unsigned int i = 0; i < nFaces; ++i) {
//The ordering is to ensure that the triangles are all drawn with the same orientation // The ordering is to ensure that the triangles are all drawn with the same orientation
if ((i + 1) % 2 == 0) { if ((i + 1) % 2 == 0) {
//For even n, vertices n + 1, n, and n + 2 define triangle n // For even n, vertices n + 1, n, and n + 2 define triangle n
SetFaceAndAdvance3(facePtr, aim->mNumVertices, data.GetUInt(i + 1), data.GetUInt(i), data.GetUInt(i + 2)); SetFaceAndAdvance3(facePtr, aim->mNumVertices, data.GetUInt(i + 1), data.GetUInt(i), data.GetUInt(i + 2));
} else { } else {
//For odd n, vertices n, n+1, and n+2 define triangle n // For odd n, vertices n, n+1, and n+2 define triangle n
SetFaceAndAdvance3(facePtr, aim->mNumVertices, data.GetUInt(i), data.GetUInt(i + 1), data.GetUInt(i + 2)); SetFaceAndAdvance3(facePtr, aim->mNumVertices, data.GetUInt(i), data.GetUInt(i + 1), data.GetUInt(i + 2));
} }
} }
@ -776,12 +779,12 @@ void glTF2Importer::ImportMeshes(glTF2::Asset &r) {
nFaces = count - 2; nFaces = count - 2;
facePtr = faces = new aiFace[nFaces]; facePtr = faces = new aiFace[nFaces];
for (unsigned int i = 0; i < nFaces; ++i) { for (unsigned int i = 0; i < nFaces; ++i) {
//The ordering is to ensure that the triangles are all drawn with the same orientation // The ordering is to ensure that the triangles are all drawn with the same orientation
if ((i + 1) % 2 == 0) { if ((i + 1) % 2 == 0) {
//For even n, vertices n + 1, n, and n + 2 define triangle n // For even n, vertices n + 1, n, and n + 2 define triangle n
SetFaceAndAdvance3(facePtr, aim->mNumVertices, i + 1, i, i + 2); SetFaceAndAdvance3(facePtr, aim->mNumVertices, i + 1, i, i + 2);
} else { } else {
//For odd n, vertices n, n+1, and n+2 define triangle n // For odd n, vertices n, n+1, and n+2 define triangle n
SetFaceAndAdvance3(facePtr, aim->mNumVertices, i, i + 1, i + 2); SetFaceAndAdvance3(facePtr, aim->mNumVertices, i, i + 1, i + 2);
} }
} }
@ -904,14 +907,14 @@ void glTF2Importer::ImportLights(glTF2::Asset &r) {
ail->mAttenuationLinear = 0.0; ail->mAttenuationLinear = 0.0;
ail->mAttenuationQuadratic = 0.0; ail->mAttenuationQuadratic = 0.0;
} else { } else {
//in PBR attenuation is calculated using inverse square law which can be expressed // in PBR attenuation is calculated using inverse square law which can be expressed
//using assimps equation: 1/(att0 + att1 * d + att2 * d*d) with the following parameters // using assimps equation: 1/(att0 + att1 * d + att2 * d*d) with the following parameters
//this is correct equation for the case when range (see // this is correct equation for the case when range (see
//https://github.com/KhronosGroup/glTF/tree/master/extensions/2.0/Khronos/KHR_lights_punctual) // https://github.com/KhronosGroup/glTF/tree/master/extensions/2.0/Khronos/KHR_lights_punctual)
//is not present. When range is not present it is assumed that it is infinite and so numerator is 1. // is not present. When range is not present it is assumed that it is infinite and so numerator is 1.
//When range is present then numerator might be any value in range [0,1] and then assimps equation // When range is present then numerator might be any value in range [0,1] and then assimps equation
//will not suffice. In this case range is added into metadata in ImportNode function // will not suffice. In this case range is added into metadata in ImportNode function
//and its up to implementation to read it when it wants to // and its up to implementation to read it when it wants to
ail->mAttenuationConstant = 0.0; ail->mAttenuationConstant = 0.0;
ail->mAttenuationLinear = 0.0; ail->mAttenuationLinear = 0.0;
ail->mAttenuationQuadratic = 1.0; ail->mAttenuationQuadratic = 1.0;
@ -1161,8 +1164,8 @@ aiNode *ImportNode(aiScene *pScene, glTF2::Asset &r, std::vector<unsigned int> &
if (node.light) { if (node.light) {
pScene->mLights[node.light.GetIndex()]->mName = ainode->mName; pScene->mLights[node.light.GetIndex()]->mName = ainode->mName;
//range is optional - see https://github.com/KhronosGroup/glTF/tree/master/extensions/2.0/Khronos/KHR_lights_punctual // range is optional - see https://github.com/KhronosGroup/glTF/tree/master/extensions/2.0/Khronos/KHR_lights_punctual
//it is added to meta data of parent node, because there is no other place to put it // it is added to meta data of parent node, because there is no other place to put it
if (node.light->range.isPresent) { if (node.light->range.isPresent) {
if (!ainode->mMetaData) { if (!ainode->mMetaData) {
ainode->mMetaData = aiMetadata::Alloc(1); ainode->mMetaData = aiMetadata::Alloc(1);
@ -1556,9 +1559,9 @@ void glTF2Importer::ImportEmbeddedTextures(glTF2::Asset &r) {
if (ext) { if (ext) {
if (strcmp(ext, "jpeg") == 0) { if (strcmp(ext, "jpeg") == 0) {
ext = "jpg"; ext = "jpg";
} else if (strcmp(ext, "ktx2") == 0) { //basisu: ktx remains } else if (strcmp(ext, "ktx2") == 0) { // basisu: ktx remains
ext = "kx2"; ext = "kx2";
} else if (strcmp(ext, "basis") == 0) { //basisu } else if (strcmp(ext, "basis") == 0) { // basisu
ext = "bu"; ext = "bu";
} }