[scudo] Add extra information when a header is corrupted.

[cferris]

Summary

When a header checksum doesn't match, it would be good to see the
two checksums to get an idea of how corrupted the header might
have become.

[cferris]

Fix typo in report.h

[Chia-hungDuan]

This is nice!

[cryptoad]

To be fair, at this point, the process should be dead and the Cookie should no longer be relevant, but from a security perspective, I would be warry about echo'ing back the expected checksum.
I understand the rationale behind, but I could think about some circumstances (forked process, VM snapshots, etc), where that information could potentially be reused to craft a correct header.
It obviously would require the attacker to get their hands on the logs first.

Overall it might not be super realistic, but leaving that here for you to judge.

[Chia-hungDuan]

In D152985#4424864, @cryptoad wrote:

To be fair, at this point, the process should be dead and the Cookie should no longer be relevant, but from a security perspective, I would be warry about echo'ing back the expected checksum.
I understand the rationale behind, but I could think about some circumstances (forked process, VM snapshots, etc), where that information could potentially be reused to craft a correct header.
It obviously would require the attacker to get their hands on the logs first.

Overall it might not be super realistic, but leaving that here for you to judge.

This is a good point! I thought the process is dead and didn't think about the cases you mentioned. That does something should be evaluated as well.

[cferris]

This is a tough one. We are already seeing cases where this extra information would really help. I'm torn because on a normal Android build you can't see the log. However, you could get a beta version and see the log. On the other hand, I think there are devices where you could probably rebuild the platform and have total control. This could definitely make things a little easier to do an attack that modifies the header and then check to see how close you are.

I'll let this sit for a bit and think about if there is a different way to get this kind of information without giving away too much about the header checksum.

[cferris]

The intent was to try and figure out whether the header was completely corrupted or it might be a bit flip. But it's not clear if that information could be actionable, so I'm just going to remove this change.

compiler-rt/lib/scudo/standalone/chunk.h

inline void storeHeader(u32 Cookie, void *Ptr,
   PackedHeader NewPackedHeader = bit_cast<PackedHeader>(*NewUnpackedHeader);
   atomic_store_relaxed(getAtomicHeader(Ptr), NewPackedHeader);
 }
 
 inline void loadHeader(u32 Cookie, const void *Ptr,
                        UnpackedHeader *NewUnpackedHeader) {
   PackedHeader NewPackedHeader = atomic_load_relaxed(getConstAtomicHeader(Ptr));
   *NewUnpackedHeader = bit_cast<UnpackedHeader>(NewPackedHeader);
-  if (UNLIKELY(NewUnpackedHeader->Checksum !=
-               computeHeaderChecksum(Cookie, Ptr, NewUnpackedHeader)))
-    reportHeaderCorruption(const_cast<void *>(Ptr));
+  u16 checksum = computeHeaderChecksum(Cookie, Ptr, NewUnpackedHeader);
+  if (UNLIKELY(NewUnpackedHeader->Checksum != checksum))
+    reportHeaderCorruption(const_cast<void *>(Ptr), NewUnpackedHeader->Checksum,
+                           checksum);
 }
 
 inline void compareExchangeHeader(u32 Cookie, void *Ptr,
                                   UnpackedHeader *NewUnpackedHeader,
                                   UnpackedHeader *OldUnpackedHeader) {
   NewUnpackedHeader->Checksum =
       computeHeaderChecksum(Cookie, Ptr, NewUnpackedHeader);
   PackedHeader NewPackedHeader = bit_cast<PackedHeader>(*NewUnpackedHeader);

compiler-rt/lib/scudo/standalone/report.h

 
 // Generic error.
 void NORETURN reportError(const char *Message);
 
 // Flags related errors.
 void NORETURN reportInvalidFlag(const char *FlagType, const char *Value);
 
 // Chunk header related errors.
-void NORETURN reportHeaderCorruption(void *Ptr);
+void NORETURN reportHeaderCorruption(void *Ptr, const u16 ExpectedChecksum,
+                                     const u16 ComputedChecksum);
 void NORETURN reportHeaderRace(void *Ptr);
 
 // Sanity checks related error.
 void NORETURN reportSanityCheckError(const char *Field);
 
 // Combined allocator errors.
 void NORETURN reportAlignmentTooBig(uptr Alignment, uptr MaxAlignment);
 void NORETURN reportAllocationSizeTooBig(uptr UserSize, uptr TotalSize,

compiler-rt/lib/scudo/standalone/report.cpp

 
 void NORETURN reportInvalidFlag(const char *FlagType, const char *Value) {
   ScopedErrorReport Report;
   Report.append("invalid value for %s option: '%s'\n", FlagType, Value);
 }
 
 // The checksum of a chunk header is invalid. This could be caused by an
 // {over,under}write of the header, a pointer that is not an actual chunk.
-void NORETURN reportHeaderCorruption(void *Ptr) {
+void NORETURN reportHeaderCorruption(void *Ptr, const u16 ExpectedChecksum,
+                                     const u16 ComputedChecksum) {
   ScopedErrorReport Report;
-  Report.append("corrupted chunk header at address %p\n", Ptr);
+  Report.append("corrupted chunk header at address %p: expected checksum: 0x%x "
+                "computed checksum: 0x%x\n",
+                Ptr, ExpectedChecksum, ComputedChecksum);
 }
 
 // Two threads have attempted to modify a chunk header at the same time. This is
 // symptomatic of a race-condition in the application code, or general lack of
 // proper locking.
 void NORETURN reportHeaderRace(void *Ptr) {
   ScopedErrorReport Report;
   Report.append("race on chunk header at address %p\n", Ptr);

compiler-rt/lib/scudo/standalone/tests/report_test.cpp

EXPECT_DEATH(CHECK_GT(-1, 1),
                "\\(u64\\)op2=1");
 }
 
 TEST(ScudoReportDeathTest, Generic) {
   // Potentially unused if EXPECT_DEATH isn't defined.
   UNUSED void *P = reinterpret_cast<void *>(0x42424242U);
   EXPECT_DEATH(scudo::reportError("TEST123"), "Scudo ERROR.*TEST123");
   EXPECT_DEATH(scudo::reportInvalidFlag("ABC", "DEF"), "Scudo ERROR.*ABC.*DEF");
-  EXPECT_DEATH(scudo::reportHeaderCorruption(P), "Scudo ERROR.*42424242");
+  EXPECT_DEATH(scudo::reportHeaderCorruption(P, 0x123, 0x124),
+               "Scudo ERROR.*42424242: expected checksum: 0x123 computed "
+               "checksum: 0x124");
   EXPECT_DEATH(scudo::reportHeaderRace(P), "Scudo ERROR.*42424242");
   EXPECT_DEATH(scudo::reportSanityCheckError("XYZ"), "Scudo ERROR.*XYZ");
   EXPECT_DEATH(scudo::reportAlignmentTooBig(123, 456), "Scudo ERROR.*123.*456");
   EXPECT_DEATH(scudo::reportAllocationSizeTooBig(123, 456, 789),
                "Scudo ERROR.*123.*456.*789");
   EXPECT_DEATH(scudo::reportOutOfMemory(4242), "Scudo ERROR.*4242");
   EXPECT_DEATH(
       scudo::reportInvalidChunkState(scudo::AllocatorAction::Recycling, P),