CVE-2017-8891: DEPRECATED: Use of Uninitialized Resource (CWE-1187)
value: Dropbox Lepton 1.2.1 allows DoS (SEGV and application crash) via a malformed lepton file because the code does not ensure setup of a correct number of threads.
CWE description: This entry has been deprecated because it was a duplicate of CWE-908. All content has been transferred to CWE-908.
CVSS v3 base score: 5.5
Published: 2017-05-10T16:29Z
Repository: https://github.com/dropbox/lepton
Language: C++
Files changed: src/lepton/bitops.cc, src/lepton/bitops.hh, src/lepton/lepton_codec.cc, src/lepton/vp8_decoder.cc, src/vp8/decoder/boolreader.hh
Commit date: 2017-04-04 23:58:01 -0700
Commit message: fix #87 : always check that threads_required set up the appropriate number of threads---fire off nop functions on unused threads for consistency

Diff: diff --git a/src/lepton/bitops.cc b/src/lepton/bitops.cc
index 3032fb1..517bdce 100644
--- a/src/lepton/bitops.cc
+++ b/src/lepton/bitops.cc
@@ -354,10 +354,11 @@ bounded_iostream::bounded_iostream(Sirikata::DecoderWriter *w,
                                    const Sirikata::JpegAllocator<uint8_t> &alloc) 
     : parent(w), err(Sirikata::JpegError::nil()) {
     this->size_callback = size_callback;
     buffer_position = 0;
     byte_position = 0;
+    byte_bound = 0x7FFFFFFF;
     num_bytes_attempted_to_write = 0;
     set_bound(0);
 }
 void bounded_iostream::call_size_callback(size_t size) {
     size_callback(parent, size);
@@ -382,11 +383,11 @@ void bounded_iostream::flush() {
 void bounded_iostream::close() {
     flush();
     parent->Close();
 }
 
-unsigned int bounded_iostream::write_no_buffer(const void *from, size_t bytes_to_write) {
+uint32_t bounded_iostream::write_no_buffer(const void *from, size_t bytes_to_write) {
     //return iostream::write(from,tpsize,dtsize);
     std::pair<unsigned int, Sirikata::JpegError> retval;
     if (byte_bound != 0 && byte_position + bytes_to_write > byte_bound) {
         size_t real_bytes_to_write = byte_bound - byte_position;
         byte_position += real_bytes_to_write;
diff --git a/src/lepton/bitops.hh b/src/lepton/bitops.hh
index 2aa36e9..9cb959c 100644
--- a/src/lepton/bitops.hh
+++ b/src/lepton/bitops.hh
@@ -465,16 +465,16 @@ class bounded_iostream
         buffer_size = 65536
     };
     uint8_t buffer[buffer_size];
     uint32_t buffer_position;
     Sirikata::DecoderWriter *parent;
-    unsigned int byte_bound;
-    unsigned int byte_position;
-    unsigned int num_bytes_attempted_to_write;
+    uint32_t byte_bound;
+    uint32_t byte_position;
+    uint32_t num_bytes_attempted_to_write;
     Sirikata::JpegError err;
     std::function<void(Sirikata::DecoderWriter*, size_t)> size_callback;
-    unsigned int write_no_buffer( const void* from, size_t bytes_to_write );
+    uint32_t write_no_buffer( const void* from, size_t bytes_to_write );
 public:
 	bounded_iostream( Sirikata::DecoderWriter * parent,
                       const std::function<void(Sirikata::DecoderWriter*, size_t)> &size_callback,
                       const Sirikata::JpegAllocator<uint8_t> &alloc);
 	~bounded_iostream( void );
diff --git a/src/lepton/lepton_codec.cc b/src/lepton/lepton_codec.cc
index cbdf10f..5442b23 100644
--- a/src/lepton/lepton_codec.cc
+++ b/src/lepton/lepton_codec.cc
@@ -273,10 +273,11 @@ CodingReturnValue LeptonCodec::ThreadState::vp8_decode_thread(unsigned int threa
                       (size_t)ColorChannel::NumBlockTypes> max_coded_heights
         = colldata->get_max_coded_heights();
     /* deserialize each block in planar order */
 
     dev_assert(luma_splits_.size() == 2); // not ready to do multiple work items on a thread yet
+    always_assert(luma_splits_.size() >= 2);
     int min_y = luma_splits_[0];
     int max_y = luma_splits_[1];
     while(true) {
         RowSpec cur_row = row_spec_from_index(decode_index_++, image_data, colldata->get_mcu_count_vertical(), max_coded_heights);
         if (cur_row.done) {
diff --git a/src/lepton/vp8_decoder.cc b/src/lepton/vp8_decoder.cc
index 412632e..8a0cace 100644
--- a/src/lepton/vp8_decoder.cc
+++ b/src/lepton/vp8_decoder.cc
@@ -415,10 +415,11 @@ std::vector<ThreadHandoff> VP8ComponentDecoder::initialize_decoder_state(const U
     return thread_handoff_;
 }
 void VP8ComponentDecoder::flush() {
         mux_splicer.drain(mux_reader_);
 }
+namespace{void nop(){}}
 CodingReturnValue VP8ComponentDecoder::decode_chunk(UncompressedComponents * const colldata)
 {
     mux_splicer.init(spin_workers_);
     /* cmpc is a global variable with the component count */
 
@@ -453,18 +454,23 @@ CodingReturnValue VP8ComponentDecoder::decode_chunk(UncompressedComponents * con
         }
     }
     if (do_threading_) {
         for (unsigned int thread_id = 0; thread_id < NUM_THREADS; ++thread_id) {
             unsigned int cur_spin_worker = thread_id;
-            spin_workers_[cur_spin_worker].work
-                = std::bind(worker_thread,
-                            thread_state_[thread_id],
-                            thread_id,
-                            colldata,
-                            mux_splicer.thread_target,
-                            getWorker(cur_spin_worker),
-                            &send_to_actual_thread_state);
+            if (!thread_state_[thread_id]) {
+                spin_workers_[cur_spin_worker].work
+                    = &nop;
+            } else {
+                spin_workers_[cur_spin_worker].work
+                    = std::bind(worker_thread,
+                                thread_state_[thread_id],
+                                thread_id,
+                                colldata,
+                                mux_splicer.thread_target,
+                                getWorker(cur_spin_worker),
+                                &send_to_actual_thread_state);
+            }
             spin_workers_[cur_spin_worker].activate_work();
         }
         flush();
         for (unsigned int thread_id = 0; thread_id < NUM_THREADS; ++thread_id) {
             unsigned int cur_spin_worker = thread_id;
diff --git a/src/vp8/decoder/boolreader.hh b/src/vp8/decoder/boolreader.hh
index 0d0d331..17a9ed6 100644
--- a/src/vp8/decoder/boolreader.hh
+++ b/src/vp8/decoder/boolreader.hh
@@ -117,11 +117,13 @@ public:
         if ((ptrdiff_t)size < rope[0].second-rope[0].first) {
             memcpy(dest, rope[0].first, size);
             return;
         }
         size_t del = rope[0].second-rope[0].first;
-        memcpy(dest, rope[0].first, del);
+        if (del) {
+            memcpy(dest, rope[0].first, del);
+        }
         dest += del;
         size -=del;
         if (size) {
             always_assert(rope[1].second - rope[1].first >= (ptrdiff_t)size);
             memcpy(dest, rope[1].first, size);

Vulnerable code: unsigned int bounded_iostream::write_no_buffer(const void *from, size_t bytes_to_write) {
    unsigned int byte_bound;
    unsigned int byte_position;
    unsigned int num_bytes_attempted_to_write;
    unsigned int write_no_buffer( const void* from, size_t bytes_to_write );
            spin_workers_[cur_spin_worker].work
                = std::bind(worker_thread,
                            thread_state_[thread_id],
                            thread_id,
                            colldata,
                            mux_splicer.thread_target,
                            getWorker(cur_spin_worker),
                            &send_to_actual_thread_state);
        memcpy(dest, rope[0].first, del);
Fixed code:     byte_bound = 0x7FFFFFFF;
uint32_t bounded_iostream::write_no_buffer(const void *from, size_t bytes_to_write) {
    uint32_t byte_bound;
    uint32_t byte_position;
    uint32_t num_bytes_attempted_to_write;
    uint32_t write_no_buffer( const void* from, size_t bytes_to_write );
    always_assert(luma_splits_.size() >= 2);
namespace{void nop(){}}
            if (!thread_state_[thread_id]) {
                spin_workers_[cur_spin_worker].work
                    = &nop;
            } else {
                spin_workers_[cur_spin_worker].work
                    = std::bind(worker_thread,
                                thread_state_[thread_id],
                                thread_id,
                                colldata,
                                mux_splicer.thread_target,
                                getWorker(cur_spin_worker),
                                &send_to_actual_thread_state);
            }
        if (del) {
            memcpy(dest, rope[0].first, del);
        }