Skip to content

Commit ff29790

Browse files
committed
[Converter:Bugfix] Fix MNNConvert crash on malformed tflite models (#4796)
MNNConvert -f TFLITE crashed with SIGSEGV on eight malformed models reported in #4796. All share the same root cause: values taken straight from the model file are used without a check that stops execution. Root causes fixed: - AsXxxOptions() returns nullptr when the builtin_options union tag does not match; the result was dereferenced unconditionally (Conv2D, DepthwiseConv2D, TransposeConv, Pool2D, FullyConnected). - DCHECK only logs and continues (logkit CHECK has no abort), so a weight shape that is not 4-D, non-positive dims, or a product that wraps in int fell through into element loads/writes. - Bias buffers were copied with memcpy without verifying the buffer holds sizeof(T)*co bytes (float/INT8/UINT8 paths). - convertDataFormatTflite's DCHECKs were no-ops; it now validates dims and src/dst and returns false instead of writing through null. - ConcatSizeComputer dereferenced inputs[0] with an empty input list and trusted a rank that can overflow the 9-slot dim array. Fix: reject the malformed operator with the converter's idiomatic dstOp->type = OpType_MAX (liteConverter.cpp turns that into a clean conversion failure), validate weight shape/size with int64 arithmetic, bound-check every bias buffer, and make convertDataFormatTflite and ConcatSizeComputer fail closed. Verified: all 8 reproducers exit cleanly (8/8 SEGV on master), a legal conv+depthwise+pool+fc+concat model converts identically to baseline (only the random model UUID differs), and the full set is clean under AddressSanitizer.
1 parent e8a3e6d commit ff29790

5 files changed

Lines changed: 171 additions & 51 deletions

File tree

source/shape/ShapeConcat.cpp

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,10 @@ class ConcatSizeComputer : public SizeComputer {
1414
virtual bool onComputeSize(const MNN::Op* op, const std::vector<Tensor*>& inputs,
1515
const std::vector<Tensor*>& outputs) const override {
1616
MNN_ASSERT(1 == outputs.size());
17-
// MNN_ASSERT(inputs.size() >= 2);
17+
if (inputs.empty()) {
18+
MNN_ERROR("Concat op has no input\n");
19+
return false;
20+
}
1821
auto& ob = outputs[0]->buffer();
1922
int basicAxis = 0;
2023
if (op->type() == OpType_Concat) {
@@ -30,6 +33,10 @@ class ConcatSizeComputer : public SizeComputer {
3033
// Concat-inputs may have scalar which should be delete
3134
for (const auto& input : inputs) {
3235
auto inputDimensions = input->buffer().dimensions;
36+
if (inputDimensions <= 0 || inputDimensions > MNN_MAX_TENSOR_DIM) {
37+
MNN_ERROR("Concat op input has invalid rank %d\n", inputDimensions);
38+
return false;
39+
}
3340

3441
// Tensor might be zeros size, but some dims may not be zero. should concat as usual.
3542

@@ -39,12 +46,20 @@ class ConcatSizeComputer : public SizeComputer {
3946
if (axis < 0) {
4047
axis = inputDimensions + axis;
4148
}
49+
if (axis < 0 || axis >= inputDimensions) {
50+
MNN_ERROR("Concat op axis %d out of range for %d dims\n", axis, inputDimensions);
51+
return false;
52+
}
4253
break;
4354
}
4455

4556

4657
int sum = 0;
4758
for (auto t : inputs) {
59+
if (axis >= t->dimensions()) {
60+
MNN_ERROR("Concat op axis %d out of range for %d dims\n", axis, t->dimensions());
61+
return false;
62+
}
4863
sum += t->buffer().dim[axis].extent;
4964
ob.type = t->buffer().type;
5065
for (int i = 0; i < t->dimensions(); ++i) {

tools/converter/source/tflite/ConvolutionTflite.cpp

Lines changed: 84 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
//
88

99
#include <stdio.h>
10+
#include <limits>
1011

1112
#include "TfliteUtils.hpp"
1213
#include "liteOpConverter.hpp"
@@ -35,6 +36,11 @@ void Conv2DTflite::run(MNN::OpT* dstOp, const std::unique_ptr<tflite::OperatorT>
3536
const int inputSize = tfliteOp->inputs.size();
3637
DCHECK(inputSize == 2 || inputSize == 3) << "tflite Conv2D input ERROR! ";
3738
const auto& tfliteConvOption = tfliteOp->builtin_options.AsConv2DOptions();
39+
if (nullptr == tfliteConvOption) {
40+
DLOG(ERROR) << "CONV_2D operator carries no Conv2DOptions";
41+
dstOp->type = MNN::OpType_MAX;
42+
return;
43+
}
3844
const int inputIndex = tfliteOp->inputs[0];
3945
const int weightIndex = tfliteOp->inputs[1];
4046
const int outputIndex = tfliteOp->outputs[0];
@@ -60,12 +66,27 @@ void Conv2DTflite::run(MNN::OpT* dstOp, const std::unique_ptr<tflite::OperatorT>
6066
int group = 1;
6167
// co kh kw ci
6268
const auto& weightShape = weightTensor->shape;
63-
DCHECK(weightShape.size() == 4) << "Conv2D weight ERROR!";
69+
if (4 != weightShape.size()) {
70+
DLOG(ERROR) << "CONV_2D weight shape is not 4-D";
71+
dstOp->type = MNN::OpType_MAX;
72+
return;
73+
}
6474
const int co = weightShape[0];
6575
const int kh = weightShape[1];
6676
const int kw = weightShape[2];
6777
const int ci = weightShape[3];
68-
const int weightSize = co * kh * kw * ci;
78+
if (co <= 0 || kh <= 0 || kw <= 0 || ci <= 0) {
79+
DLOG(ERROR) << "CONV_2D weight shape contains non-positive dimension";
80+
dstOp->type = MNN::OpType_MAX;
81+
return;
82+
}
83+
const int64_t weightSize64 = (int64_t)co * kh * kw * ci;
84+
if (weightSize64 <= 0 || weightSize64 > std::numeric_limits<int>::max()) {
85+
DLOG(ERROR) << "CONV_2D weight size overflow: " << co << "x" << kh << "x" << kw << "x" << ci;
86+
dstOp->type = MNN::OpType_MAX;
87+
return;
88+
}
89+
const int weightSize = (int)weightSize64;
6990
if (inputShape.size() == 4 && inputShape[3] > ci) {
7091
group = inputShape[3] / ci;
7192
}
@@ -160,11 +181,14 @@ void Conv2DTflite::run(MNN::OpT* dstOp, const std::unique_ptr<tflite::OperatorT>
160181
conv2dParamQuan->biasQuantizedParam = std::unique_ptr<MNN::QuantizedParamT>(new MNN::QuantizedParamT);
161182
conv2dParamQuan->biasQuantizedParam->zeroPoint = biasTensor->quantization->zero_point[0];
162183
conv2dParamQuan->biasQuantizedParam->scale = biasTensor->quantization->scale[0];
163-
DCHECK(biasData.size() / 4 == co) << "Bias Data ERROR";
164-
auto biasDataPtr = biasData.data();
165-
const int32_t* realBiasDataPtr = (int32_t*)biasDataPtr;
166-
std::vector<int32_t> biasInt32Vec(realBiasDataPtr, realBiasDataPtr + co);
167-
conv2dParamQuan->bias = biasInt32Vec;
184+
if (biasData.size() >= sizeof(int32_t) * co) {
185+
auto biasDataPtr = biasData.data();
186+
const int32_t* realBiasDataPtr = (int32_t*)biasDataPtr;
187+
std::vector<int32_t> biasInt32Vec(realBiasDataPtr, realBiasDataPtr + co);
188+
conv2dParamQuan->bias = biasInt32Vec;
189+
} else {
190+
DLOG(ERROR) << "CONV_2D bias buffer is too small, ignore bias";
191+
}
168192
}
169193

170194
conv2dParamQuan->activationType = (MNN::FusedActivation)tfliteConvOption->fused_activation_function;
@@ -255,10 +279,15 @@ void Conv2DTflite::run(MNN::OpT* dstOp, const std::unique_ptr<tflite::OperatorT>
255279
convolution2DQuant->bias.resize(co);
256280
if (inputSize == 3) {
257281
const auto& biasTensor = tfliteTensors[tfliteOp->inputs[2]];
258-
auto bias = reinterpret_cast<const int*>(tfliteModelBuffer[biasTensor->buffer]->data.data());
259-
// int to float
260-
for (int i = 0; i < co; i++) {
261-
convolution2DQuant->bias[i] = bias[i] * (scaleIn * alpha[i]);
282+
const auto& biasRaw = tfliteModelBuffer[biasTensor->buffer]->data;
283+
auto bias = reinterpret_cast<const int*>(biasRaw.data());
284+
if (biasRaw.size() >= sizeof(int) * co) {
285+
// int to float
286+
for (int i = 0; i < co; i++) {
287+
convolution2DQuant->bias[i] = bias[i] * (scaleIn * alpha[i]);
288+
}
289+
} else {
290+
DLOG(ERROR) << "CONV_2D bias buffer is too small, ignore bias";
262291
}
263292
}
264293
dstOp->main.value = convolution2DQuant.release();
@@ -323,8 +352,12 @@ void Conv2DTflite::run(MNN::OpT* dstOp, const std::unique_ptr<tflite::OperatorT>
323352
std::vector<float> biasData(co, 0.0f);
324353
if (inputSize == 3) {
325354
const auto& biasTensor = tfliteTensors[tfliteOp->inputs[2]];
326-
auto biasDataPtr = reinterpret_cast<const float*>(tfliteModelBuffer[biasTensor->buffer]->data.data());
327-
::memcpy(biasData.data(), biasDataPtr, sizeof(float) * co);
355+
const auto& biasRaw = tfliteModelBuffer[biasTensor->buffer]->data;
356+
if (biasRaw.data() != nullptr && biasRaw.size() >= sizeof(float) * co) {
357+
::memcpy(biasData.data(), biasRaw.data(), sizeof(float) * co);
358+
} else {
359+
DLOG(ERROR) << "CONV_2D bias buffer is too small, ignore bias";
360+
}
328361
}
329362
convolution2DFloat->bias = biasData;
330363
dstOp->main.value = convolution2DFloat.release();
@@ -365,32 +398,58 @@ void TransposeConvTflite::run(MNN::OpT *dstOp, const std::unique_ptr<tflite::Ope
365398
}
366399
*/
367400
const auto& tfliteConvOption = tfliteOp->builtin_options.AsTransposeConvOptions();
401+
if (nullptr == tfliteConvOption) {
402+
DLOG(ERROR) << "TRANSPOSE_CONV operator carries no TransposeConvOptions";
403+
dstOp->type = MNN::OpType_MAX;
404+
return;
405+
}
368406
// weight index
369407
const int weightIndex = tfliteOp->inputs[1];
370408
const auto& weightTensor = tfliteTensors[weightIndex];
371409
// co kh kw ci
372410
const auto& weightShape = weightTensor->shape;
373-
DCHECK(weightShape.size() == 4) << "Conv2D weight ERROR!";
411+
if (4 != weightShape.size()) {
412+
DLOG(ERROR) << "TRANSPOSE_CONV weight shape is not 4-D";
413+
dstOp->type = MNN::OpType_MAX;
414+
return;
415+
}
374416
const int co = weightShape[0];
375417
const int kh = weightShape[1];
376418
const int kw = weightShape[2];
377419
const int ci = weightShape[3];
378-
const int weightSize = co * kh * kw * ci;
420+
if (co <= 0 || kh <= 0 || kw <= 0 || ci <= 0) {
421+
DLOG(ERROR) << "TRANSPOSE_CONV weight shape contains non-positive dimension";
422+
dstOp->type = MNN::OpType_MAX;
423+
return;
424+
}
425+
const int64_t weightSize64 = (int64_t)co * kh * kw * ci;
426+
if (weightSize64 <= 0 || weightSize64 > std::numeric_limits<int>::max()) {
427+
DLOG(ERROR) << "TRANSPOSE_CONV weight size overflow: " << co << "x" << kh << "x" << kw << "x" << ci;
428+
dstOp->type = MNN::OpType_MAX;
429+
return;
430+
}
431+
const int weightSize = (int)weightSize64;
379432
{
380433
auto convolution2DFloat = new MNN::Convolution2DT;
381434
// weight
382435
std::vector<float> weightData;
383436
weightData.resize(weightSize);
384437
auto originalWeightPtr = reinterpret_cast<const float*>(tfliteModelBuffer[weightTensor->buffer]->data.data());
385-
convertDataFormatTflite(originalWeightPtr, weightData.data(), kh, kw, ci, co, true);
438+
if (!convertDataFormatTflite(originalWeightPtr, weightData.data(), kh, kw, ci, co, true)) {
439+
DLOG(ERROR) << "TRANSPOSE_CONV weight data is invalid";
440+
dstOp->type = MNN::OpType_MAX;
441+
return;
442+
}
386443
convolution2DFloat->weight = weightData;
387444
// bias
388445
std::vector<float> biasData(co, 0.0f);
389446
if (inputSize == 4) {
390447
const auto& biasTensor = tfliteTensors[tfliteOp->inputs[2]];
391-
auto biasDataPtr = reinterpret_cast<const float*>(tfliteModelBuffer[biasTensor->buffer]->data.data());
392-
if(biasDataPtr){
393-
::memcpy(biasData.data(), biasDataPtr, sizeof(float) * co);
448+
const auto& biasRaw = tfliteModelBuffer[biasTensor->buffer]->data;
449+
if (biasRaw.data() != nullptr && biasRaw.size() >= sizeof(float) * co) {
450+
::memcpy(biasData.data(), biasRaw.data(), sizeof(float) * co);
451+
} else {
452+
DLOG(ERROR) << "TRANSPOSE_CONV bias buffer is too small, ignore bias";
394453
}
395454
}
396455
convolution2DFloat->bias = biasData;
@@ -440,11 +499,16 @@ void FullConnectedTflite::run(MNN::OpT* dstOp, const std::unique_ptr<tflite::Ope
440499
const std::vector<std::unique_ptr<tflite::TensorT>>& tfliteTensors,
441500
const std::vector<std::unique_ptr<tflite::BufferT>>& tfliteModelBuffer,
442501
const std::vector<std::unique_ptr<tflite::OperatorCodeT>>& tfliteOpSet, int quantizedModel) {
502+
const auto& option = tfliteOp->builtin_options.AsFullyConnectedOptions();
503+
if (nullptr == option) {
504+
DLOG(ERROR) << "FULLY_CONNECTED operator carries no FullyConnectedOptions";
505+
dstOp->type = MNN::OpType_MAX;
506+
return;
507+
}
443508
dstOp->main.value = new MNN::ExtraT;
444509
auto dstP = dstOp->main.AsExtra();
445510
dstP->engine = "Tflite";
446511
dstP->type = "FULL_CONNECT";
447-
const auto& option = tfliteOp->builtin_options.AsFullyConnectedOptions();
448512
dstP->attr.resize(3);
449513
dstP->attr[0].reset(new MNN::AttributeT);
450514
dstP->attr[0]->key = "keep_num_dims";

tools/converter/source/tflite/DepthwiseConv2DTflite.cpp

Lines changed: 58 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
//
88

99
#include <stdio.h>
10+
#include <limits>
1011

1112
#include "TfliteUtils.hpp"
1213
#include "liteOpConverter.hpp"
@@ -78,13 +79,33 @@ void DepthwiseConv2DTflite::run(MNN::OpT* dstOp, const std::unique_ptr<tflite::O
7879
const auto& weightTensor = tfliteTensors[weightIndex];
7980
// co kh kw ci
8081
const auto& weightShape = weightTensor->shape;
81-
DCHECK(weightShape.size() == 4) << "Conv2D weight ERROR!";
82+
if (4 != weightShape.size()) {
83+
DLOG(ERROR) << "DEPTHWISE_CONV_2D weight shape is not 4-D";
84+
dstOp->type = MNN::OpType_MAX;
85+
return;
86+
}
8287
// const int co = weightShape[0];
8388
const int kh = weightShape[1];
8489
const int kw = weightShape[2];
8590
const int ci = weightShape[3];
86-
const int weightSize = kh * kw * ci;
91+
if (kh <= 0 || kw <= 0 || ci <= 0) {
92+
DLOG(ERROR) << "DEPTHWISE_CONV_2D weight shape contains non-positive dimension";
93+
dstOp->type = MNN::OpType_MAX;
94+
return;
95+
}
96+
const int64_t weightSize64 = (int64_t)kh * kw * ci;
97+
if (weightSize64 <= 0 || weightSize64 > std::numeric_limits<int>::max()) {
98+
DLOG(ERROR) << "DEPTHWISE_CONV_2D weight size overflow: " << kh << "x" << kw << "x" << ci;
99+
dstOp->type = MNN::OpType_MAX;
100+
return;
101+
}
102+
const int weightSize = (int)weightSize64;
87103
const auto& tfliteConvOption = tfliteOp->builtin_options.AsDepthwiseConv2DOptions();
104+
if (nullptr == tfliteConvOption) {
105+
DLOG(ERROR) << "DEPTHWISE_CONV_2D operator carries no DepthwiseConv2DOptions";
106+
dstOp->type = MNN::OpType_MAX;
107+
return;
108+
}
88109
if (weightTensor->type == tflite::TensorType_INT8) {
89110
quantizedModel = 2;
90111
dstOp->type = MNN::OpType_ConvolutionDepthwise;
@@ -115,30 +136,37 @@ void DepthwiseConv2DTflite::run(MNN::OpT* dstOp, const std::unique_ptr<tflite::O
115136
::memset(depthwiseConv2dParamFloat->bias.data(), 0, outputCount * sizeof(float));
116137
if (inputSize == 3) {
117138
const auto& biasTensor = tfliteTensors[tfliteOp->inputs[2]];
118-
if (biasTensor->quantization->scale.size() == 1) {
119-
auto scale = biasTensor->quantization->scale[0];
120-
auto zero = biasTensor->quantization->zero_point[0];;
121-
const auto& biasData = tfliteModelBuffer[biasTensor->buffer]->data;
122-
auto biasDataPtr = biasData.data();
123-
const int32_t* realBiasDataPtr = (int32_t*)biasDataPtr;
124-
for (int i=0; i<outputCount; ++i) {
125-
depthwiseConv2dParamFloat->bias[i] = (float)(realBiasDataPtr[i] - zero) * scale;
139+
const auto& biasData = tfliteModelBuffer[biasTensor->buffer]->data;
140+
if (biasData.size() >= sizeof(int32_t) * outputCount) {
141+
if (biasTensor->quantization->scale.size() == 1) {
142+
auto scale = biasTensor->quantization->scale[0];
143+
auto zero = biasTensor->quantization->zero_point[0];;
144+
auto biasDataPtr = biasData.data();
145+
const int32_t* realBiasDataPtr = (int32_t*)biasDataPtr;
146+
for (int i=0; i<outputCount; ++i) {
147+
depthwiseConv2dParamFloat->bias[i] = (float)(realBiasDataPtr[i] - zero) * scale;
148+
}
149+
} else {
150+
auto biasDataPtr = biasData.data();
151+
const int32_t* realBiasDataPtr = (int32_t*)biasDataPtr;
152+
for (int i=0; i<outputCount; ++i) {
153+
depthwiseConv2dParamFloat->bias[i] = (float)(realBiasDataPtr[i] - biasTensor->quantization->zero_point[i]) * biasTensor->quantization->scale[i];
154+
}
126155
}
127156
} else {
128-
const auto& biasData = tfliteModelBuffer[biasTensor->buffer]->data;
129-
auto biasDataPtr = biasData.data();
130-
const int32_t* realBiasDataPtr = (int32_t*)biasDataPtr;
131-
for (int i=0; i<outputCount; ++i) {
132-
depthwiseConv2dParamFloat->bias[i] = (float)(realBiasDataPtr[i] - biasTensor->quantization->zero_point[i]) * biasTensor->quantization->scale[i];
133-
}
157+
DLOG(ERROR) << "DEPTHWISE_CONV_2D bias buffer is too small, ignore bias";
134158
}
135159
}
136160
// Weight
137161
// Transpose first
138162
std::vector<int8_t> transposeWeight(kw * kh * ci);
139163
const auto& weightData = tfliteModelBuffer[weightTensor->buffer]->data;
140164
auto weightDataPtr = (int8_t*)weightData.data();
141-
165+
if (weightDataPtr == nullptr || weightData.size() < (size_t)kw * kh * ci) {
166+
DLOG(ERROR) << "DEPTHWISE_CONV_2D INT8 weight buffer is too small";
167+
dstOp->type = MNN::OpType_MAX;
168+
return;
169+
}
142170
for (int i=0; i<ci; ++i) {
143171
for (int j=0; j<kw*kh; ++j) {
144172
transposeWeight[i*kw*kh+j] = weightDataPtr[i+j*ci];
@@ -193,11 +221,14 @@ void DepthwiseConv2DTflite::run(MNN::OpT* dstOp, const std::unique_ptr<tflite::O
193221

194222
auto shape = biasTensor->shape;
195223

196-
DCHECK(biasData.size() / 4 == ci) << "Bias Data ERROR";
197-
auto biasDataPtr = biasData.data();
198-
const int32_t* realBiasDataPtr = (int32_t*)biasDataPtr;
199-
std::vector<int32_t> biasInt32Vec(realBiasDataPtr, realBiasDataPtr + ci);
200-
depthwiseConv2dParamQuan->bias = biasInt32Vec;
224+
if (biasData.size() >= sizeof(int32_t) * ci) {
225+
auto biasDataPtr = biasData.data();
226+
const int32_t* realBiasDataPtr = (int32_t*)biasDataPtr;
227+
std::vector<int32_t> biasInt32Vec(realBiasDataPtr, realBiasDataPtr + ci);
228+
depthwiseConv2dParamQuan->bias = biasInt32Vec;
229+
} else {
230+
DLOG(ERROR) << "DEPTHWISE_CONV_2D bias buffer is too small, ignore bias";
231+
}
201232
}
202233
depthwiseConv2dParamQuan->activationType =
203234
static_cast<MNN::FusedActivation>(tfliteConvOption->fused_activation_function);
@@ -216,11 +247,13 @@ void DepthwiseConv2DTflite::run(MNN::OpT* dstOp, const std::unique_ptr<tflite::O
216247
// bias
217248
if (inputSize == 3) {
218249
const auto& biasTensor = tfliteTensors[tfliteOp->inputs[2]];
219-
auto originalBiasPtr = reinterpret_cast<const float*>(tfliteModelBuffer[biasTensor->buffer]->data.data());
220-
if (originalBiasPtr) {
250+
const auto& biasRaw = tfliteModelBuffer[biasTensor->buffer]->data;
251+
if (biasRaw.data() != nullptr && biasRaw.size() >= sizeof(float) * ci) {
221252
std::vector<float> biasData(ci, 0.0f);
222-
::memcpy(biasData.data(), originalBiasPtr, sizeof(float) * ci);
253+
::memcpy(biasData.data(), biasRaw.data(), sizeof(float) * ci);
223254
depthwiseConv2dParamFloat->bias = biasData;
255+
} else {
256+
DLOG(ERROR) << "DEPTHWISE_CONV_2D bias buffer is too small, ignore bias";
224257
}
225258
}
226259
depthwiseConv2dParamFloat->common = std::move(dstCommon);

tools/converter/source/tflite/PoolingTflite.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,11 @@ void PoolingTflite::run(MNN::OpT* dstOp, const std::unique_ptr<tflite::OperatorT
2828
const std::vector<std::unique_ptr<tflite::BufferT>>& tfliteModelBuffer,
2929
const std::vector<std::unique_ptr<tflite::OperatorCodeT>>& tfliteOpSet, int quantizedModel) {
3030
const auto& tflitePoolOption = tfliteOp->builtin_options.AsPool2DOptions();
31+
if (nullptr == tflitePoolOption) {
32+
DLOG(ERROR) << "AVERAGE_POOL_2D/MAX_POOL_2D operator carries no Pool2DOptions";
33+
dstOp->type = MNN::OpType_MAX;
34+
return;
35+
}
3136
const int outputIndex = tfliteOp->outputs[0];
3237
const auto& outputTensor = tfliteTensors[outputIndex];
3338
if (outputTensor->type == tflite::TensorType_INT8) {

0 commit comments

Comments
 (0)