src: audit and fix shift-into-sign-bit bugs

$ git grep -n  '<< 24' src/
src/base64.h:88:        unbase64(src[i + 0]) << 24 |
src/cares_wrap.cc:83:  return static_cast<uint32_t>(p[0] << 24U) |
src/node_sockaddr.cc:305:  uint32_t check = ptr[0] << 24 | ptr[1] << 16 | ptr[2] << 8 | ptr[3];
src/util-inl.h:51:  (((x) & 0xFF) << 24) |                                                      \
src/util-inl.h:61:  (((x) & 0x0000000000FF0000ull) << 24) |

The LHS here is usually of type unsigned char but gets promoted to int (because 24 is an int) and is then left-shifted.

If the LHS >= 128, that ends up shifting into the sign bit and that's implementation-defined behavior (i.e., bad - although it probably works okay with most compilers.)

Replace 24 with 24u and all is good. But! It's probably best to abstract it away into a helper function.

cares_wrap.cc already has one - cares_get_32bit() - that handles it correctly.

Replace 24 with 24U, git grep -n '<< 24' src/, nodejs#34827

Quick question, why can't cares_get_32bit() be used or considered as that helper (like here)? That function could work with

src/base64.h:88:        unbase64(src[i + 0]) << 24 |
src/cares_wrap.cc:83:  return static_cast<uint32_t>(p[0] << 24U) |
src/node_sockaddr.cc:305:  uint32_t check = ptr[0] << 24 | ptr[1] << 16 | ptr[2] << 8 | ptr[3];

@juanarbol

@juanarbol It's okay to rename and move that function to e.g. src/util.h.

Ok, I'll be working on this.

I don't find a way to

  inline static int8_t unbase64(uint8_t x);

  // Send all this as a const unsigned char pointer
  // Or convert all this elements into a const unsigned char pointer
  unbase64(src[i + 0]);
  unbase64(src[i + 1]);
  unbase64(src[i + 2]);
  unbase64(src[i + 3]);

  // and send it to our beautiful method
  cares_get_32bit(&mentioned_above_const_char);

src: create helper for reading Uint32BE

Navigation Menu

Repository navigation

src: audit and fix shift-into-sign-bit bugs #34827

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

src: audit and fix shift-into-sign-bit bugs #34827

Description

Activity

juanarbol commented on Aug 24, 2020

bnoordhuis commented on Aug 24, 2020

juanarbol commented on Aug 24, 2020

juanarbol commented on Aug 25, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions