Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: Added the worse NCCE I could think of, and the least abominable way to turn it into a CS

...

This noncompliant code example runs in kernel space and copies data from testarg to user space. However, padding bits may be used within the object, for example, to ensure the proper alignment of class data members. These padding bits may contain sensitive information, which may then be leaked when the data is copied to user space.

...

Code Block
bgColor#ccccff
langcpp
#include <cstddef>
#include <cstring>
 
struct test {
  int a;
  char b;
  int c;
};
 
// Safely copy bytes to user space.
extern int copy_to_user(void *dest, void *src, std::size_t size);
 
void do_stuff(void *usr_buf) {
  test arg{1, 2, 3};
  // May be larger than strictly needed.
  unsigned char buf[sizeof(arg)];
  std::size_t offset = 0;
 
  std::memcpy(buf + offset, &arg.a, sizeof(arg.a));
  offset += sizeof(arg.a);
  std::memcpy(buf + offset, &arg.b, sizeof(arg.b));
  offset += sizeof(arg.b);
  std::memcpy(buf + offset, &arg.c, sizeof(arg.c));
  offset += sizeof(arg.c);
 
  copy_to_user(usr_buf, buf, offset /* size of info copied */);
}

...

Code Block
bgColor#ccccff
langcpp
#include <cstddef>

struct test {
  int a;
  char b;
  char padding_1, padding_2, padding_3;
  int c;
 
  test(int a, char b, int c) : a(a), b(b),
    padding_1(0), padding_2(0), padding_3(0),
    c(c) {}
};
// Ensure c is the next byte after the last padding byte.
static_assert(offsetof(test, c) == offsetof(test, padding_3) + 1,
              "Object contains intermediate padding");
// Ensure there is no trailing padding.
static_assert(sizeof(test) == offsetof(test, c) + sizeof(int),
              "Object contains trailing padding");



// Safely copy bytes to user space.
extern int copy_to_user(void *dest, void *src, std::size_t size);

void do_stuff(void *usr_buf) {
  test arg{1, 2, 3};
  copy_to_user(usr_buf, &arg, sizeof(arg));
}

The static_assert() declaration accepts a constant expression and an error message. The expression is evaluated at compile time and, if false, the compilation is terminated and the error message is used as the diagnostic. The explicit insertion of the padding bytes into the struct should ensure that no additional padding bytes are added by the compiler, and consequently both static assertions should be true. However, it is necessary to validate these assumptions to ensure that the solution is correct for a particular implementation.

...

hiddentrue

Noncompliant Code Example

In this noncompliant code example, padding bits may abound, including when:

  • the virtual method table requires additional padding bits for aligning a subsequent data member,
  • alignment padding bits are required to position a subsequent data member on a properly-aligned boundary,
  • bit-field padding bits are required because the sequential set of bit-fields does not fill an entire allocation unit,
  • a bit-field is declared with a length greater than the number of bits in the underlying allocation unit (the extra bits are treated as padding bits),
  • the class instance itself requires additional padding bits to ensure it will be appropriately aligned for use within an array, or
  • padding bits are required due to the allocation of data members of varying access control levels.

This code example runs in kernel space and copies data from arg to user space. However, the padding bits within the object instance may contain sensitive information that will then be leaked when the data is copied to user space.

Code Block
bgColor#FFcccc
langcpp
#include <cstddef>

class base {
public:
  virtual ~base() = default;
};

class test : public virtual base {
  alignas(32) double h;
  char i;
  unsigned j : 80;
protected:
  unsigned k;
  unsigned l : 4;
public:
  char m;
  double n;
  
  test(double h, char i, unsigned j, unsigned k, unsigned l, char m, double n) :
    h(h), i(i), j(j), k(k), l(l), m(m), n(n) {}
  
  virtual void foo();
};

// Safely copy bytes to user space.
extern int copy_to_user(void *dest, void *src, std::size_t size);

void do_stuff(void *usr_buf) {
  test arg{0.0, 1, 2, 3, 4, 5, 6.0};
  copy_to_user(usr_buf, &arg, sizeof(arg));
}

When compiled with GCC 5.3.0 for x86-32, the test object requires 96 bytes of storage to accommodate 27 bytes of data (31 bytes including the vtable) and has the following layout:

Offset (bytes (bits))Storage Size (bytes (bits))Reason OffsetStorage SizeReason
01 (32)vtable pointer 56 (448)4 (32)unsigned k
4 (32)28 (224)data member alignment padding 60 (480)0 (4)unsigned l : 4
32 (256)8 (64)double h 60 (484)0 (4)unused bit-field bits
40 (320)1 (8)char i 61 (488)1 (8)char m
41 (328)3 (24)data member alignment padding 62 (496)2 (16)data member alignment padding
44 (352)4 (32)unsigned j : 80 64 (512)8 (64)double n
48 (384)6 (48)extended bit-field size padding 72 (576)24 (192)class alignment padding
54 (432)2 (16)alignment padding    

Compliant Solution

Due to the complexity of the data structure, this compliant solution serializes the object data before copying it to an untrusted context instead of attempting to account for all of the padding bytes manually:

Code Block
bgColor#ccccff
langcpp
#include <cstddef>
#include <cstring>
 
class base {
public:
  virtual ~base() = default;
};
class test : public virtual base {
  alignas(32) double h;
  char i;
  unsigned j : 80;
protected:
  unsigned k;
  unsigned l : 4;
public:
  char m;
  double n;
  
  test(double h, char i, unsigned j, unsigned k, unsigned l, char m, double n) :
    h(h), i(i), j(j), k(k), l(l), m(m), n(n) {}
  
  virtual void foo();
  bool serialize(unsigned char *buffer, std::size_t &size) {
    if (size < sizeof(test)) {
      return false;
    }
    
    std::size_t offset = 0;
    std::memcpy(buffer + offset, &h, sizeof(h));
    offset += sizeof(h);
    std::memcpy(buffer + offset, &i, sizeof(i));
    offset += sizeof(i);
    unsigned loc_j = j; // Only sizeof(unsigned) bits are valid, so this is not narrowing.
    std::memcpy(buffer + offset, &loc_j, sizeof(loc_j));
    offset += sizeof(loc_j);
    std::memcpy(buffer + offset, &k, sizeof(k));
    offset += sizeof(k);
    unsigned char loc_l = l & 0b1111;
    std::memcpy(buffer + offset, &loc_l, sizeof(loc_l));
    offset += sizeof(loc_l);
    std::memcpy(buffer + offset, &m, sizeof(m));
    offset += sizeof(m);
    std::memcpy(buffer + offset, &n, sizeof(n));
    offset += sizeof(n);
    
    size -= offset;
    return true;
  }
};
 
// Safely copy bytes to user space.
extern int copy_to_user(void *dest, void *src, size_t size);
 
void do_stuff(void *usr_buf) {
  test arg{0.0, 1, 2, 3, 4, 5, 6.0};
  
  // May be larger than strictly needed, will be updated by
  // calling serialize() to the size of the buffer remaining.
  std::size_t size = sizeof(arg);
  unsigned char buf[size];
  if (arg.serialize(buf, size)) {
    copy_to_user(usr_buf, buf, sizeof(test) - size);
  } else {
    // Handle error
  }
}

This code ensures that no uninitialized padding bits are copied to unprivileged users. Note that the structure copied to user space is now a packed structure and that the copy_to_user() function would need to unpack it to re-create the original, padded structure

...

.

Risk Assessment

Padding bits might inadvertently contain sensitive data such as pointers to kernel data structures or passwords. A pointer to such a structure could be passed to other functions, causing information leakage.

...