Skip to content

Commit abd1d7d

Browse files
committed
This closes issue #30. Thanks to [Arron](https://github.com/boxerab) for highlighting this issue.
Now there is additional 8 bytes in front of the codeblock buffer at the decoder. This extra buffer is a protection against the VLC decoder reading from before the start of the cleanup pass, which can happen because we are reading 4 bytes from the VLC segment at a time. Additional improvements: 1. Bug fix in ojph_block_encoder. 2. Hardened the code by not decoding incomplete codeblocks.
1 parent a27e7d6 commit abd1d7d

File tree

4 files changed

+22
-12
lines changed

4 files changed

+22
-12
lines changed

src/core/codestream/ojph_codestream.cpp

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2939,12 +2939,8 @@ namespace ojph {
29392939
int avail_bits;
29402940
bool unstuff;
29412941
int bytes_left;
2942-
static const int extra_buffer_space;
29432942
};
29442943

2945-
//////////////////////////////////////////////////////////////////////////
2946-
const int bit_read_buf::extra_buffer_space = 8;
2947-
29482944
//////////////////////////////////////////////////////////////////////////
29492945
static inline
29502946
void bb_init(bit_read_buf *bbp, int bytes_left, infile_base* file)
@@ -3019,11 +3015,13 @@ namespace ojph {
30193015
{
30203016
assert(bbp->avail_bits == 0 && bbp->unstuff == false);
30213017
int bytes = ojph_min(num_bytes, bbp->bytes_left);
3022-
elastic->get_buffer(bytes + bit_read_buf::extra_buffer_space,
3023-
cur_coded_list);
3024-
size_t bytes_read = bbp->file->read(cur_coded_list->buf, bytes);
3018+
elastic->get_buffer(bytes + coded_cb_header::prefix_buf_size
3019+
+ coded_cb_header::suffix_buf_size, cur_coded_list);
3020+
size_t bytes_read = bbp->file->read(
3021+
cur_coded_list->buf + coded_cb_header::prefix_buf_size, bytes);
30253022
if (num_bytes > bytes_read)
3026-
memset(cur_coded_list->buf + bytes, 0, num_bytes - bytes_read);
3023+
memset(cur_coded_list->buf + coded_cb_header::prefix_buf_size + bytes,
3024+
0, num_bytes - bytes_read);
30273025
bbp->bytes_left -= bytes_read;
30283026
return bytes_read == bytes;
30293027
}
@@ -3270,7 +3268,14 @@ namespace ojph {
32703268
int num_bytes = cp->pass_length[0] + cp->pass_length[1];
32713269
if (num_bytes)
32723270
if (!bb_read_chunk(&bb, num_bytes, cp->next_coded, elastic))
3273-
{ data_left = bb.bytes_left; assert(data_left == 0); return; }
3271+
{
3272+
//no need to decode a broken codeblock, decoding is a
3273+
// security risk
3274+
cp->pass_length[0] = cp->pass_length[1] = 0;
3275+
data_left = bb.bytes_left;
3276+
assert(data_left == 0);
3277+
return;
3278+
}
32743279
}
32753280
}
32763281
}
@@ -3689,9 +3694,10 @@ namespace ojph {
36893694
//////////////////////////////////////////////////////////////////////////
36903695
void codeblock::decode()
36913696
{
3692-
if (coded_cb->num_passes > 0)
3697+
if (coded_cb->pass_length[0] > 0 && coded_cb->num_passes > 0)
36933698
{
3694-
ojph_decode_codeblock(coded_cb->next_coded->buf,
3699+
ojph_decode_codeblock(
3700+
coded_cb->next_coded->buf + coded_cb_header::prefix_buf_size,
36953701
buf, coded_cb->missing_msbs, coded_cb->num_passes,
36963702
coded_cb->pass_length[0], coded_cb->pass_length[1],
36973703
cb_size.w, cb_size.h, cb_size.w);

src/core/codestream/ojph_codestream_local.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,9 @@ namespace ojph {
353353
int Kmax;
354354
int missing_msbs;
355355
coded_lists *next_coded;
356+
357+
static const int prefix_buf_size = 8;
358+
static const int suffix_buf_size = 8;
356359
};
357360

358361
}

src/core/coding/ojph_block_encoder.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -713,6 +713,7 @@ namespace ojph {
713713
rho[0] = rho[1] = 0; e_qmax[0] = e_qmax[1] = 0;
714714
}
715715

716+
lep[1] = 0;
716717

717718
for (y = 2; y < height; y += 2)
718719
{

src/core/common/ojph_defs.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ typedef int64_t si64;
5858
/////////////////////////////////////////////////////////////////////////////
5959
#define OJPH_CORE_VER_MAJOR 0
6060
#define OJPH_CORE_VER_MINOR 6
61-
#define OJPH_CORE_VER_SUBMINOR 3
61+
#define OJPH_CORE_VER_SUBMINOR 4
6262

6363
/////////////////////////////////////////////////////////////////////////////
6464
#define OJPH_INT_STRINGIFY(I) #I

0 commit comments

Comments
 (0)