This template is ONLY for reporting security bugs. If you are reporting a
Download Protection Bypass bug, please use the "Security - Download
Protection" template. For all other reports, please use a different
template.

Please see the following link for instructions on filing security bugs:
http://www.chromium.org/Home/chromium-security/reporting-security-bugs


VULNERABILITY DETAILS

Several instances of potential double-reads in the GPU command buffer service code. For the builds that I have checked, it seems that the compiler generates safe code (linux x64, chromeos ARM, windows x86), to various degrees - the windows code is the closest to being vulnerable, probably due to limited registers on x86; the windows build does have double-reads in most of these places, but missing the most interesting double-read on args.data_memory_size in HandleGetBucketStart, probably due to the ordering of the || statement. Most of the other issues require the unsafe_es3_apis_enabled flag, so don't affect default configuration.

I'm not 100% sure that the compiler is anyway required to generate safe code for any of the GPU command buffer handler functions even with the use of local variables; I'm not a compiler expert, but I suspect that the command structures should be declared volatile const & before reading struct members into locals, or explicitly memcpy'd out of shared memory into local copies, so that the compiler knows that it is not ok to assume it can just read the unchanged values back out of the shared memory page.

Reporting without applying deadline since these all appear to be "safe" in the official chrome production builds that I have checked, but with security flag since this may not be true for all chromium builds, especially if someone is still releasing a 32-bit linux chromium build...

--------------------------------------------------------------------------------

error::Error CommonDecoder::HandleGetBucketStart(uint32_t immediate_data_size,
                                                 const void* cmd_data) {
  const cmd::GetBucketStart& args =
      *static_cast<const cmd::GetBucketStart*>(cmd_data);
  uint32_t bucket_id = args.bucket_id;
  uint32_t* result = GetSharedMemoryAs<uint32_t*>(
      args.result_memory_id, args.result_memory_offset, sizeof(*result));
  int32_t data_memory_id = args.data_memory_id;
  uint32_t data_memory_offset = args.data_memory_offset;
  uint32_t data_memory_size = args.data_memory_size;
  uint8_t* data = NULL;
  if (data_memory_size != 0 || data_memory_id != 0 || data_memory_offset != 0) {
    data = GetSharedMemoryAs<uint8_t*>(
        args.data_memory_id, args.data_memory_offset, args.data_memory_size); // <---- double-read on args.data_memory_id, args.data_memory_offset and args.data_memory_size
    if (!data) {
      return error::kInvalidArguments;
    }
  }
  if (!result) {
    return error::kInvalidArguments;
  }
  // Check that the client initialized the result.
  if (*result != 0) {
    return error::kInvalidArguments;
  }
  Bucket* bucket = GetBucket(bucket_id);
  if (!bucket) {
    return error::kInvalidArguments;
  }
  uint32_t bucket_size = bucket->size();
  *result = bucket_size;
  if (data) {
    uint32_t size = std::min(data_memory_size, bucket_size);
    memcpy(data, bucket->GetData(0, size), size); // <---- size could be out of sync here if args.data_memory_size is reduced in between the two reads
  }
  return error::kNoError;
}

--------------------------------------------------------------------------------

error::Error GLES2DecoderImpl::HandleVertexAttribIPointer(
    uint32_t immediate_data_size,
    const void* cmd_data) {
  if (!unsafe_es3_apis_enabled())
    return error::kUnknownCommand;
  const gles2::cmds::VertexAttribIPointer& c =
      *static_cast<const gles2::cmds::VertexAttribIPointer*>(cmd_data);

  if (!state_.bound_array_buffer.get() ||
      state_.bound_array_buffer->IsDeleted()) {
    if (state_.vertex_attrib_manager.get() ==
        state_.default_vertex_attrib_manager.get()) {
      LOCAL_SET_GL_ERROR(
          GL_INVALID_OPERATION,
          "glVertexAttribIPointer", "no array buffer bound");
      return error::kNoError;
    } else if (c.offset != 0) {
      LOCAL_SET_GL_ERROR(
          GL_INVALID_OPERATION,
          "glVertexAttribIPointer", "client side arrays are not allowed");
      return error::kNoError;
    }
  }

  GLuint indx = c.indx;
  GLint size = c.size;
  GLenum type = c.type;
  GLsizei stride = c.stride;
  GLsizei offset = c.offset; // <---- possible double-read

--------------------------------------------------------------------------------

error::Error GLES2DecoderImpl::HandleVertexAttribPointer(
    uint32_t immediate_data_size,
    const void* cmd_data) {
  const gles2::cmds::VertexAttribPointer& c =
      *static_cast<const gles2::cmds::VertexAttribPointer*>(cmd_data);

  if (!state_.bound_array_buffer.get() ||
      state_.bound_array_buffer->IsDeleted()) {
    if (state_.vertex_attrib_manager.get() ==
        state_.default_vertex_attrib_manager.get()) {
      LOCAL_SET_GL_ERROR(
          GL_INVALID_OPERATION,
          "glVertexAttribPointer", "no array buffer bound");
      return error::kNoError;
    } else if (c.offset != 0) {
      LOCAL_SET_GL_ERROR(
          GL_INVALID_OPERATION,
          "glVertexAttribPointer", "client side arrays are not allowed");
      return error::kNoError;
    }
  }

  GLuint indx = c.indx;
  GLint size = c.size;
  GLenum type = c.type;
  GLboolean normalized = static_cast<GLboolean>(c.normalized);
  GLsizei stride = c.stride;
  GLsizei offset = c.offset; // <---- possible double-read


error::Error GLES2DecoderImpl::HandleWaitSyncTokenCHROMIUM(
    uint32_t immediate_data_size,
    const void* cmd_data) {
  const gles2::cmds::WaitSyncTokenCHROMIUM& c =
      *static_cast<const gles2::cmds::WaitSyncTokenCHROMIUM*>(cmd_data);

  const gpu::CommandBufferNamespace kMinNamespaceId =
      gpu::CommandBufferNamespace::INVALID;
  const gpu::CommandBufferNamespace kMaxNamespaceId =
      gpu::CommandBufferNamespace::NUM_COMMAND_BUFFER_NAMESPACES;

  const gpu::CommandBufferNamespace namespace_id =
      ((c.namespace_id >= static_cast<int32_t>(kMinNamespaceId)) &&
       (c.namespace_id < static_cast<int32_t>(kMaxNamespaceId)))      // <---- possible double-read
          ? static_cast<gpu::CommandBufferNamespace>(c.namespace_id)  // <---- possible double-read
          : gpu::CommandBufferNamespace::INVALID;


error::Error GLES2DecoderImpl::HandleMapBufferRange(
    uint32_t immediate_data_size, const void* cmd_data) {
  if (!unsafe_es3_apis_enabled()) {
    return error::kUnknownCommand;
  }
  const gles2::cmds::MapBufferRange& c =
      *static_cast<const gles2::cmds::MapBufferRange*>(cmd_data);
  GLenum target = static_cast<GLenum>(c.target);
  GLbitfield access = static_cast<GLbitfield>(c.access);
  GLintptr offset = static_cast<GLintptr>(c.offset);
  GLsizeiptr size = static_cast<GLsizeiptr>(c.size);

  typedef cmds::MapBufferRange::Result Result;
  Result* result = GetSharedMemoryAs<Result*>(
      c.result_shm_id, c.result_shm_offset, sizeof(*result));
  if (!result) {
    return error::kOutOfBounds;
  }
  if (*result != 0) {
    *result = 0;
    return error::kInvalidArguments;
  }
  int8_t* mem =
      GetSharedMemoryAs<int8_t*>(c.data_shm_id, c.data_shm_offset, size);
  if (!mem) {
    return error::kOutOfBounds;
  }

  GLbitfield mask = GL_MAP_INVALIDATE_BUFFER_BIT;
  if ((access & mask) == mask) {
    // TODO(zmo): To be on the safe side, always map
    // GL_MAP_INVALIDATE_BUFFER_BIT to GL_MAP_INVALIDATE_RANGE_BIT.
    access = (access & ~GL_MAP_INVALIDATE_BUFFER_BIT);
    access = (access | GL_MAP_INVALIDATE_RANGE_BIT);
  }
  // TODO(zmo): Always filter out GL_MAP_UNSYNCHRONIZED_BIT to get rid of
  // undefined behaviors.
  mask = GL_MAP_READ_BIT | GL_MAP_UNSYNCHRONIZED_BIT;
  if ((access & mask) == mask) {
    LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, "MapBufferRange",
                       "incompatible access bits");
    return error::kNoError;
  }
  access = (access & ~GL_MAP_UNSYNCHRONIZED_BIT);
  if ((access & GL_MAP_WRITE_BIT) == GL_MAP_WRITE_BIT &&
      (access & GL_MAP_INVALIDATE_RANGE_BIT) == 0) {
    access = (access | GL_MAP_READ_BIT);
  }
  void* ptr = glMapBufferRange(target, offset, size, access);
  if (ptr == nullptr) {
    return error::kNoError;
  }
  Buffer* buffer = buffer_manager()->GetBufferInfoForTarget(&state_, target);
  DCHECK(buffer);
  buffer->SetMappedRange(offset, size, access, ptr,
                         GetSharedMemoryBuffer(c.data_shm_id)); // <---- possible double-read


VERSION
Chrome Version: latest
Operating System: [Please indicate OS, version, and service pack level]

REPRODUCTION CASE
Please include a demonstration of the security bug, such as an attached
HTML or binary file that reproduces the bug when loaded in Chrome. PLEASE
make the file as small as possible and remove any content not required to
demonstrate the bug.

FOR CRASHES, PLEASE INCLUDE THE FOLLOWING ADDITIONAL INFORMATION
Type of crash: [tab, browser, etc.]
Crash State: [see link above: stack trace, registers, exception record]
Client ID (if relevant): [see link above]