CATEGORY: MITRE CWE-188: Reliance on Data/Memory Layout. REFERENCE: https://cwe.mitre.org/data/definitions/188.html EXPLANATION: The code is written under the assumption that there are no padding bytes within 'measurement_payload_t', but most likely, there is a padding byte after member 'abs_humidity' to achieve proper memory alignment of subsequent members. Padding bytes of global objects, that is, objects allocated statically in the data or const data segment will typically (but are not guaranteed to) have a value of zero. Padding bytes of objects allocated at run-time (allocated on the stack or heap) have arbitrary values. Comparing measurements using 'memcmp' compares flat memory, that is, members as well as any potential padding bytes. 'measurement.payload' is a copy of an object that was allocated on the stack and 'INVALID_MEASUREMENT' is an object alloced in the const data segment; they will thus very likely have different padding byte values. CONSEQUENCES: For all practical purposes, in 'process_env_measurement', 'memcmp' will indicate that the two measurement objects are different, even if all members have identical (ie. invalid) values. Therefore, all-invalid measurements will be processed, even though they should be ignored. BUGFIX: Since the value of padding bytes is unspecified (ref. C99/C11 6.2.6.1) the only safe/portable way to compare these struct instances is to compare all members individually, thus skipping padding bytes: /* If there is not even a single valid measurement. */ if ( measurement.payload.abs_humidity == INVALID_MEASUREMENT.abs_humidity && measurement.payload.temperature == INVALID_MEASUREMENT.temperature && measurement.payload.bar_pressure == INVALID_MEASUREMENT.bar_pressure && measurement.payload.timestamp == INVALID_MEASUREMENT.timestamp) { /* Ignore this measurement. */ ... REMARKS: (1) If one has control over the initialization of a struct object (which is not the case for objects located in the const data segment), it is possible to force the value of padding bytes to a well-defined value via 'memset': struct measurement_payload_t my_measurement; /* Initialize object, including padding bytes. */ memset(&my_measurement, 0, sizeof(g_invalid_measurement)); /* Fill in rest of members. */ ... Comparing such fully initialized objects via 'memcmp' will then be both, safe and fast. (2) Computing hashes over unspecified values can also be problematic, especially if hashes are compared later to find out if objects are identical. While it is not a problem in this particular example, it is generally wise to zero-initialize objects (incl. padding bytes) explicitly via 'memset' before calculating hashes over them. (3) Struct padding is nicely explained in this article: http://www.catb.org/esr/structure-packing/