CERT
Skip to end of metadata
Go to start of metadata

Bitwise operators include the complement operator ~, bitwise shift operators >> and <<, bitwise AND operator &, bitwise exclusive OR operator ^, bitwise inclusive OR operator | and compound assignment operators >>=, <<=, &=, ^= and |=. Bitwise operators should be used only with unsigned integer operands, as the results of bitwise operations on signed integers are implementation-defined.

The C11 standard, section 6.5, paragraph 4 [ISO/IEC 9899:2011], states:

Some operators (the unary operator ~ , and the binary operators <<, >>, &, ^, and |, collectively described as bitwise operators) shall have operands that have integral type. These operators return values that depend on the internal representations of integers, and thus have implementation-defined and undefined aspects for signed types.

Furthermore, the bitwise shift operators << and >> are undefined under many circumstances, and are implementation-defined for signed integers for more circumstances; see rule INT34-C. Do not shift an expression by a negative number of bits or by greater than or equal to the number of bits that exist in the operand for more information.

Implementation details

The Microsoft C compiler documentation says that:

Bitwise operations on signed integers work the same as bitwise operations on unsigned integers.

On-line GCC documentation about the implementation of bitwise operations on signed integers says:

Bitwise operators act on the representation of the value including both the sign and value bits, where the sign bit is considered immediately above the highest-value value bit.

Noncompliant Code Example (Right Shift)

The right-shift operation may be implemented as either an arithmetic (signed) shift or a logical (unsigned) shift. If E1 in the expression E1 >> E2 has a signed type and a negative value, the resulting value is implementation-defined. Also, a bitwise shift can result in undefined behavior. (See INT34-C. Do not shift an expression by a negative number of bits or by greater than or equal to the number of bits that exist in the operand.)

This noncompliant code example can result in an error condition on implementations in which an arithmetic shift is performed, and the sign bit is propagated as the number is shifted [Dowd 2006]:

In this example, stringify >> 24 evaluates to 0xFFFFFF80, or 4,294,967,168. When converted to a string, the resulting value "4294967168" is too large to store in buf and is truncated by snprintf().

If this code had been implemented using sprintf() instead of snprintf(), this noncompliant code example would have resulted in a buffer overflow.

Compliant Solution (Right Shift)

In this compliant solution, stringify is declared as an unsigned integer. The value of the result of the right-shift operation is the integral part of the quotient of stringify / 2 ^ 24:

Also, consider using the sprintf_s() function, defined in ISO/IEC TR 24731-1, instead of snprintf() to provide some additional checks. (See STR07-C. Use the bounds-checking interfaces for string manipulation.)

Exceptions

INT13-C-EX1: When used as bit flags, it is acceptable to use preprocessor macros or enumeration constants as arguments to the & and | operators even if the value is not explicitly declared as unsigned.

INT13-C-EX2: If the right-side operand to a shift operator is known at compile time, it is acceptable for the value to be represented with a signed type provided it is positive.

Risk Assessment

Performing bitwise operations on signed numbers can lead to buffer overflows and the execution of arbitrary code by an attacker in some cases, unexpected or implementation-defined behavior in others.

Recommendation

Severity

Likelihood

Remediation Cost

Priority

Level

INT13-C

High

Unlikely

Medium

P6

L2

Automated Detection

Tool

Version

Checker

Description

CodeSonar4.4LANG.TYPE.IOTInappropriate operand type
Compass/ROSE

 

 

Can detect violations of this rule. In particular, it flags bitwise operations that involved variables not declared with unsigned type

ECLAIR

1.2

CC2.INT13

Fully implemented

Klocwork2017MISRA.BITS.NOT_UNSIGNED
MISRA.BITS.NOT_UNSIGNED.PREP
 
LDRA tool suite9.5.6

50 S
120 S
331 S

Fully implemented

Parasoft C/C++test9.5MISRA2008-5_0_21Fully implemented
PRQA QA-C9.24532, 4533, 4534, 4543, 4544Fully implemented
Splint3.1.1

 

 

Related Vulnerabilities

Search for vulnerabilities resulting from the violation of this rule on the CERT website.

Related Guidelines

SEI CERT C++ Coding StandardINT13-CPP. Use bitwise operators only on unsigned operands
ISO/IEC TR 24772:2013Bit Representations [STR]
Arithmetic Wrap-around Error [FIF]
Sign Extension Error [XZI]
MITRE CWECWE-682, Incorrect calculation

Bibliography

[Dowd 2006]Chapter 6, "C Language Issues"
[C99 Rationale 2003]Subclause 6.5.7, "Bitwise Shift Operators"

 


13 Comments

  1. The code used in the example:

    is asking to confuse readers - this has nothing to do with 256 - a common buffer limit - but instead represents the space required to store three characters plus a trailing NUL. I guarantee this will create a double-take.

    Wouldn't it illustrate the point better as:

     or (better), just

    ?


    1. To me this looks clearer as is.  It makes explicit the fact that the maximum number to be converted is 256.

  2. Sure there are. If a variable is signed (maybe due to some API or data structure you are using) and you need it shifted, you might as well do x <<= 2 or whatever and be done with it. [Edit: If you check properly that you dont' shift in or out of the sign anyway.  If you don't check, unsigned shifting behaves better.]

    You can copy it to unsigned and shift, but then you need to assign it back to signed, which means you must test that it's in range for that anyway. It might be less work to test that the signed value is in range for shifting instead.

    Also one little trap: Shifting "signed items" happens when you shift an unsigned variable narrower than int. The variable is promoted to (signed) int, and that's what you shift.

    1. The real issue is that left shifting a signed value leads to implementation-defined behavior. This is never a good thing to have...

       The easiest way to guard against this (and make it tool detectable) is to prohibit shifting of signed objects.

  3. Because this is a recommendation, I think it is reasonable to recommend that "bitwise operators be used with unsigned operands". I think perhaps we can preserve the current example, including the discussion of logical vs. arithmetic shift, and add a left shift example discussing the implementaiton defined behavior.

    1. Hm. The title is definitely too strong. For unsigned ints it is perfectly safe to assume logical & arithmetic shift. I believe the same is true for unsigned short or char (as long as you don't potentially lose data casting the result back to a short/char)

      The problem is strictly with signed integers (of all sizes), and then you're still OK as long as your domain number is nonnegative, and your output number doesn't exceed the size limit of your signed type, potentially twiddling the sign bit.

      Hallvard's instances of using shifts on signed integers don't convince me...they are operationally identicial to converting to unsigned, shifting, and converting back (including range checks)

      1. "Hallvard's instances of using shifts on signed integers don't convince me...they are operationally identicial to converting to unsigned, shifting, and converting back (including range checks)"

        Yes, that was my point. Likely applying this rule to int x; ... x >>= 10; will simply mean replacing it with something like x = (int) ((unsigned) x >> 10);. This is less readable and doesn't gain anything that I can see: Instead of an "unsafe" shift you have an "unsafe" conversion to int - and the test to ensure a correct and safe result will likely be exactly the same.

         [edit] Which doesn't mean I think this is a bad recommendation, just that you asked for counterexamples and I can think of some.  But then, even the ruleagainst using uninitialized data has useful counterexamples - as in the recent debacle where OpenSSL uses uninitialized data as one source of randomness and Debian "fixed" that code.

        About operating on narrower unsigneds than int, I'm just pointing out that when you do that, you do in fact operate on a signed value. Is this rule intended to apply to the operands before or after the integer promotions / usual arithmetic conversions? A shift example where it matters is possible but silly: Given a 16-bit short and 32-bit int, and unsigned short x; you can do x << 20 which shifts it as an int - but that can then shift into the sign bit. More relevant in this case is ~x which actually does ~(int)x.

        BTW, I had only skimmed this page before -- I note the example's "int stringify = 0x80000000;" has implementation-defined/undefined behavior, regardless of any shifting done afterwards. (Assuming 32-bit int, which the example does.)

  4. I've changed the title and scope of this rule and it is now ready for review. I did not add a second example, because I thought the rule was fairly self-explanatory although it is still a little vague in places, for example, "the results of some bitwise operations on signed integers is implementation-defined".

  5. The exceptions to this rule arise because most predefined bitflags are not explicitly defined as unsigned int (such as the bitflags to fopen() illustrated in INT13A-EX1). These exceptions do allow one to create insecure code; their presence provides 'backwards compatibility' with the standard C headers defining these bitflags. New bitflag constants should be explicitly defined as unsigned int.

  6. One could even mask the extended bits before use. The original value will be shifted as required but it will still protect the buffer overflow because of the masking. Interestingly, in Java a CS can be to use the logical shift operator ">>>" as opposed to ">>" but in C there is only one kind of operator, the ">>".
    EDIT: On closer inspection this seems to fall under INT13-EX2.

  7. compliant solution (Right Shift) has no color. how can we edit the bgcolor?

    (I cannot find any clue to change bgcolor on editing code block macro window...)

     

    1. I've fixed it – the color was set properly by the editor, but it wasn't being honored by the renderer. This happens sometimes, and the only way I've found to fix it is to delete the code block entirely and copy/paste in a working one. Thank you for pointing it out, though!

      1. Thanks, Aaron! I didn't think of copy-and-pasting the working block...