project-chip / connectedhomeip

Matter (formerly Project CHIP) creates more connections between more objects, simplifying development for manufacturers and increasing compatibility for consumers, guided by the Connectivity Standards Alliance.
https://buildwithmatter.com
Apache License 2.0
7.24k stars 1.92k forks source link

Fortify error on Android Pie #12326

Open milezhacks opened 2 years ago

milezhacks commented 2 years ago

Problem

When running CHIP on Android Pie, I'm hitting the following error:

FORTIFY: sendto: prevented 40-byte read from 24-byte buffer

I've traced this to the call getifaddrs(&mAddrsList) on https://github.com/project-chip/connectedhomeip/blob/master/src/inet/InetInterface.cpp#L653

The implementation for this function (on Android) comes from: https://github.com/project-chip/connectedhomeip/blob/master/src/inet/InetInterface.cpp#L54

I did NOT encounter this error on Android N. Incidentally, the Fortify check for send()/sendto() was added in Android O: https://android.googlesource.com/platform/bionic/+/master/docs/status.md

Upon reviewing the implementation for getifaddrs() in third_party/android/platform-libcore/android-platform-libcore/ifaddrs-android.h line 182, it looks like the size of the buffer is swollen by the padding and alignment macros.

Incidentally, bionic has provided its own getifaddrs() since at least Android L.

Proposed Solution

Either:

  1. add a max-version check to InetInterface.cpp to NOT use libcore after a certain point
  2. OR just use unconditionally
milezhacks commented 2 years ago

Here's the diff: https://github.com/project-chip/connectedhomeip/compare/master...milezhacks:issues/12326?expand=1

Will submit PR once CI completes.

milezhacks commented 2 years ago

Incidentally, bionic has provided its own getifaddrs() since at least Android L

This statement is wrong. getifaddrs() was added to Bionic in Android Nougat. For Android L (API Level 21) this stub is, in fact, required but shouldn't be used for API levels >= 24.

andy31415 commented 2 years ago

Added looks critical: FORTIFY error points to invalid memory access.