Skip to content

Commit 164b45a

Browse files
fabienfl-orcfabienfl-orc
committed
OrcLib: BinaryBuffer: add multiple null deref checks
1 parent 690666a commit 164b45a

File tree

2 files changed

+24
-147
lines changed

2 files changed

+24
-147
lines changed

‎src/OrcLib/BinaryBuffer.cpp

+20-3
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ CBinaryBuffer::CBinaryBuffer(const CBinaryBuffer& other)
4848
, m_size(0L)
4949
, m_bOwnMemory(true)
5050
, m_bVirtualAlloc(other.m_bVirtualAlloc)
51+
, m_bJunk(true)
5152
{
5253
if (other.m_size > 0)
5354
{
@@ -160,13 +161,19 @@ HRESULT CBinaryBuffer::SetData(LPCBYTE pBuffer, size_t cbSize)
160161
{
161162
if (!SetCount(cbSize))
162163
return E_OUTOFMEMORY;
164+
163165
CopyMemory(m_pData, pBuffer, cbSize);
164166
m_bJunk = false;
165167
return S_OK;
166168
}
167169

168170
HRESULT CBinaryBuffer::CopyTo(LPBYTE pBuffer, size_t cbSize)
169171
{
172+
if (m_pData == nullptr)
173+
{
174+
return S_OK;
175+
}
176+
170177
size_t to_copy = min(m_size, cbSize);
171178
CopyMemory(pBuffer, m_pData, to_copy);
172179
return S_OK;
@@ -212,18 +219,18 @@ void CBinaryBuffer::ZeroMe()
212219

213220
void CBinaryBuffer::RemoveAll()
214221
{
215-
if (m_bOwnMemory)
222+
if (m_bOwnMemory && m_pData)
216223
{
217224
if (m_bVirtualAlloc)
218225
{
219226
VirtualFree(m_pData, 0L, MEM_RELEASE);
220227
}
221228
else
222229
{
223-
if (m_pData != nullptr)
224-
HeapFree(GetBinaryBufferHeap(), 0L, m_pData);
230+
HeapFree(GetBinaryBufferHeap(), 0L, m_pData);
225231
}
226232
}
233+
227234
m_pData = nullptr;
228235
m_size = 0L;
229236
m_bOwnMemory = true;
@@ -232,6 +239,11 @@ void CBinaryBuffer::RemoveAll()
232239

233240
HRESULT CBinaryBuffer::GetSHA1(CBinaryBuffer& SHA1)
234241
{
242+
if (m_pData == nullptr)
243+
{
244+
return S_OK;
245+
}
246+
235247
if (g_hProv == NULL)
236248
if (!CryptAcquireContext(&g_hProv, NULL, NULL, PROV_RSA_FULL, CRYPT_VERIFYCONTEXT))
237249
return HRESULT_FROM_WIN32(GetLastError());
@@ -262,6 +274,11 @@ HRESULT CBinaryBuffer::GetSHA1(CBinaryBuffer& SHA1)
262274

263275
HRESULT CBinaryBuffer::GetMD5(CBinaryBuffer& MD5)
264276
{
277+
if (m_pData == nullptr)
278+
{
279+
return S_OK;
280+
}
281+
265282
if (g_hProv == NULL)
266283
if (!CryptAcquireContext(&g_hProv, NULL, NULL, PROV_RSA_FULL, CRYPT_VERIFYCONTEXT))
267284
return HRESULT_FROM_WIN32(GetLastError());

‎tests/OrcLibTest/binary_buffer_test.cpp

+4-144
Original file line numberDiff line numberDiff line change
@@ -36,294 +36,154 @@ TEST_CLASS(BinaryBufferTest)
3636

3737
TEST_METHOD(BinaryBufferBasicTest)
3838
{
39-
4039
LogLine();
4140

4241
// check buffer allocation
4342
{
44-
LogLine();
4543
CBinaryBuffer buffer;
46-
47-
LogLine();
4844
buffer.SetCount(100);
4945

50-
LogLine();
5146
Assert::IsTrue(buffer.OwnsBuffer());
52-
53-
LogLine();
5447
Assert::IsTrue(buffer.GetCount() == 100);
55-
56-
LogLine();
5748
Assert::IsTrue(buffer.CheckCount(150));
58-
59-
LogLine();
6049
Assert::IsTrue(buffer.GetCount() == 150);
6150
}
6251

6352
// binary buffer with data + md5 + sha1 + operator[]
6453
{
65-
LogLine();
6654
unsigned char data[5] = {0xAA, 0xBB, 0xCC, 0xDD, 0xEE};
6755
CBinaryBuffer buffer(data, sizeof(data));
6856

69-
LogLine();
7057
Assert::IsTrue(!buffer.OwnsBuffer());
71-
72-
LogLine();
7358
Assert::IsTrue(buffer.GetCount() == sizeof(data));
74-
75-
LogLine();
7659
Assert::IsTrue(buffer.GetData() != nullptr);
77-
78-
LogLine();
7960
Assert::IsTrue(buffer[0] == 0xAA);
80-
81-
LogLine();
8261
Assert::IsTrue(buffer[1] == 0xBB);
83-
84-
LogLine();
8562
Assert::IsTrue(buffer[2] == 0xCC);
86-
87-
LogLine();
8863
Assert::IsTrue(buffer[3] == 0xDD);
89-
90-
LogLine();
9164
Assert::IsTrue(buffer[4] == 0xEE);
9265

93-
LogLine();
9466
unsigned char md5Result[16] = {
9567
0x37, 0xD3, 0xA1, 0x9F, 0xF4, 0x69, 0x7F, 0x91, 0xFD, 0xC4, 0xC3, 0xA0, 0x69, 0x0A, 0xD8, 0xC5};
9668
CBinaryBuffer md5;
97-
98-
LogLine();
9969
buffer.GetMD5(md5);
100-
101-
LogLine();
10270
Assert::IsTrue(md5.GetCount() == sizeof(md5Result));
103-
104-
LogLine();
10571
Assert::IsTrue(!memcmp(md5.GetData(), md5Result, sizeof(md5Result)));
10672

107-
LogLine();
10873
unsigned char sha1Result[20] = {0xD5, 0xB4, 0x12, 0x31, 0x9A, 0x66, 0xBE, 0x9D, 0xCB, 0x40,
10974
0xEA, 0x0E, 0xE0, 0x9B, 0xB7, 0x62, 0x71, 0xC5, 0xE7, 0x48};
11075
CBinaryBuffer sha1;
111-
112-
LogLine();
11376
buffer.GetSHA1(sha1);
114-
115-
LogLine();
11677
Assert::IsTrue(sha1.GetCount() == sizeof(sha1Result));
117-
118-
LogLine();
11978
Assert::IsTrue(!memcmp(sha1.GetData(), sha1Result, sizeof(sha1Result)));
12079

121-
LogLine();
12280
buffer.ZeroMe();
123-
124-
LogLine();
12581
Assert::IsTrue(buffer[0] == 0x00);
126-
127-
LogLine();
12882
Assert::IsTrue(buffer[1] == 0x00);
129-
130-
LogLine();
13183
Assert::IsTrue(buffer[2] == 0x00);
132-
133-
LogLine();
13484
Assert::IsTrue(buffer[3] == 0x00);
135-
136-
LogLine();
13785
Assert::IsTrue(buffer[4] == 0x00);
13886

13987
// try [] with an invalid index
14088
bool gotException = false;
14189
try
14290
{
143-
LogLine();
14491
buffer[5];
14592
}
14693
catch (...)
14794
{
148-
LogLine();
14995
gotException = true;
15096
}
15197

152-
LogLine();
15398
Assert::IsTrue(gotException);
15499
}
155100

156101
// check copy constructor of a binary buffer
157102
{
158-
LogLine();
159-
CBinaryBuffer buffer;
160103

161-
LogLine();
104+
CBinaryBuffer buffer;
162105
buffer.SetCount(100);
163-
164-
LogLine();
165106
buffer[0] = 0xFF;
166107

167-
LogLine();
168108
CBinaryBuffer sha1;
169-
170-
LogLine();
171109
buffer.GetSHA1(sha1);
172110

173-
LogLine();
174111
CBinaryBuffer buffer2(buffer); // copy constructor (buffer owns its memory)
175-
176-
LogLine();
177112
Assert::IsTrue(buffer2.OwnsBuffer());
178-
179-
LogLine();
180113
Assert::IsTrue(buffer2.GetCount() == 100);
181-
182-
LogLine();
183114
Assert::IsTrue(buffer2[0] == 0xFF);
184115

185-
LogLine();
186116
CBinaryBuffer sha1ToCheck;
187-
188-
LogLine();
189117
buffer2.GetSHA1(sha1ToCheck);
190118

191-
LogLine();
192119
Assert::IsTrue(!memcmp(sha1.GetData(), sha1ToCheck.GetData(), sha1.GetCount()));
193120
}
194121

195122
{
196-
LogLine();
197123
unsigned char data[5] = {0xAA, 0xBB, 0xCC, 0xDD, 0xEE};
198124
CBinaryBuffer buffer(data, sizeof(data));
199-
200-
LogLine();
201125
CBinaryBuffer buffer2(buffer); // copy constructor (buffer doesn't own its memory)
202-
203-
LogLine();
204126
Assert::IsTrue(!buffer2.OwnsBuffer());
205-
206-
LogLine();
207127
Assert::IsTrue(buffer2.GetCount() == 5);
208128
}
209129

210130
// check operator= of a binary buffer
211131
{
212-
LogLine();
213132
CBinaryBuffer buffer;
214-
215-
LogLine();
216133
buffer.SetCount(100);
217-
218-
LogLine();
219134
bool ownsMemory = buffer.OwnsBuffer();
220135

221-
LogLine();
222136
size_t size = buffer.GetCount();
223-
224-
LogLine();
225137
BYTE* pointer = buffer.GetData();
226138

227-
LogLine();
228139
CBinaryBuffer buffer2;
229-
230-
LogLine();
231140
buffer2 = buffer; // operator=
232141

233-
LogLine();
234142
Assert::IsTrue(buffer2.OwnsBuffer() == ownsMemory);
235-
236-
LogLine();
237143
Assert::IsTrue(buffer2.GetCount() == size);
238-
239-
LogLine();
240144
Assert::IsTrue(buffer2.GetData() != pointer);
241-
242-
LogLine();
243145
Assert::IsTrue(buffer.OwnsBuffer() == true);
244-
245-
LogLine();
246146
Assert::IsTrue(buffer.GetCount() == size);
247-
248-
LogLine();
249147
Assert::IsTrue(buffer.GetData() != nullptr);
250148
}
251149

252150
// check move constructor / move assignment operator
253151
{
254-
LogLine();
255-
CBinaryBuffer buffer;
256152

257-
LogLine();
153+
CBinaryBuffer buffer;
258154
buffer.SetCount(100);
259-
260-
LogLine();
261155
bool ownsMemory = buffer.OwnsBuffer();
262-
263-
LogLine();
264156
size_t size = buffer.GetCount();
265157

266-
LogLine();
267158
BYTE* pointer = buffer.GetData();
268159

269160
typedef std::function<CBinaryBuffer(CBinaryBuffer)> Functor;
270161

271-
LogLine();
272162
Functor functor = [](CBinaryBuffer buffer) { return buffer; };
273163

274-
LogLine();
275164
CBinaryBuffer buffer1;
276-
277-
LogLine();
278165
buffer1 = functor(buffer); // copy constructor is called first then move assignment operator
279-
280-
LogLine();
281166
Assert::IsTrue(buffer1.OwnsBuffer() == ownsMemory);
282-
283-
LogLine();
284167
Assert::IsTrue(buffer1.GetCount() == size);
285-
286-
LogLine();
287168
Assert::IsTrue(buffer1.GetData() != pointer);
288-
289-
LogLine();
290169
Assert::IsTrue(buffer.OwnsBuffer() == ownsMemory);
291-
292-
LogLine();
293170
Assert::IsTrue(buffer.GetCount() == size);
294-
295-
LogLine();
296171
Assert::IsTrue(buffer.GetData() == pointer);
297172

298-
LogLine();
299173
ownsMemory = buffer1.OwnsBuffer(); // buffer1 should own its memory
300-
301-
LogLine();
302174
size = buffer1.GetCount();
303-
304-
LogLine();
305175
pointer = buffer1.GetData();
306176

307-
LogLine();
308177
CBinaryBuffer buffer2 = std::move(buffer1); // move constructor
309-
310-
LogLine();
311178
Assert::IsTrue(buffer2.OwnsBuffer() == ownsMemory);
312-
313-
LogLine();
314179
Assert::IsTrue(buffer2.GetCount() == size);
315-
316-
LogLine();
317180
Assert::IsTrue(buffer2.GetData() == pointer);
318-
319-
LogLine();
320-
Assert::IsTrue(buffer1.OwnsBuffer() == true);
321-
322-
LogLine();
181+
Assert::IsTrue(buffer1.OwnsBuffer() == true); // check move ctor
323182
Assert::IsTrue(buffer1.GetCount() == 0);
324183

325184
LogLine();
326185
Assert::IsTrue(buffer1.GetData() == nullptr);
186+
LogLine();
327187
}
328188
}
329189
};

0 commit comments

Comments
 (0)