Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 6bb12824 authored by Sally Qi's avatar Sally Qi Committed by Alec Mouri
Browse files

Add security check to getPhysicalDisplayToken binder function. 

- There is a possible way to take over the screen display and swap the
  display content due to a missing permission check.
- Add a short-term fix for WCG checking failure because of new
  permission check added to SF::getPhysicalDisplayToken: change two
  function signatures (getStaticDisplayInfo and getDynamicDisplayInfo).
- To make short-term fix workable, split getDynamicDisplayInfo binder
  call into two, one is to take display id, one is to take display token
  as old codes show to avoid huge modification on other callees.

Bug: 248031255
Test: test using displaytoken app manually on the phone, test shell
screenrecord during using displaytoken; atest
android.hardware.camera2.cts.FastBasicsTest

Change-Id: Id9d9012d4ede9c8330f0ce1096bcb78e51b7c5df
parent a84abc85

libs/gui/LayerState.cpp

+21 −0
Original line number Diff line number Diff line
@@ -388,6 +388,27 @@ void DisplayState::merge(const DisplayState& other) { 
    }

}



void DisplayState::sanitize(int32_t permissions) {

    if (what & DisplayState::eLayerStackChanged) {

        if (!(permissions & layer_state_t::Permission::ACCESS_SURFACE_FLINGER)) {

            what &= ~DisplayState::eLayerStackChanged;

            ALOGE("Stripped attempt to set eLayerStackChanged in sanitize");

        }

    }

    if (what & DisplayState::eDisplayProjectionChanged) {

        if (!(permissions & layer_state_t::Permission::ACCESS_SURFACE_FLINGER)) {

            what &= ~DisplayState::eDisplayProjectionChanged;

            ALOGE("Stripped attempt to set eDisplayProjectionChanged in sanitize");

        }

    }

    if (what & DisplayState::eSurfaceChanged) {

        if (!(permissions & layer_state_t::Permission::ACCESS_SURFACE_FLINGER)) {

            what &= ~DisplayState::eSurfaceChanged;

            ALOGE("Stripped attempt to set eSurfaceChanged in sanitize");

        }

    }

}



void layer_state_t::sanitize(int32_t permissions) {

    // TODO: b/109894387

    //

libs/gui/SurfaceComposerClient.cpp

+68 −49
Original line number Diff line number Diff line
@@ -2260,12 +2260,12 @@ status_t SurfaceComposerClient::getDisplayState(const sp<IBinder>& display, 
    return statusTFromBinderStatus(status);

}



status_t SurfaceComposerClient::getStaticDisplayInfo(const sp<IBinder>& display,

status_t SurfaceComposerClient::getStaticDisplayInfo(int64_t displayId,

                                                     ui::StaticDisplayInfo* outInfo) {

    using Tag = android::gui::DeviceProductInfo::ManufactureOrModelDate::Tag;

    gui::StaticDisplayInfo ginfo;

    binder::Status status =

            ComposerServiceAIDL::getComposerService()->getStaticDisplayInfo(display, &ginfo);

            ComposerServiceAIDL::getComposerService()->getStaticDisplayInfo(displayId, &ginfo);

    if (status.isOk()) {

        // convert gui::StaticDisplayInfo to ui::StaticDisplayInfo

        outInfo->connectionType = static_cast<ui::DisplayConnectionType>(ginfo.connectionType);
@@ -2309,12 +2309,8 @@ status_t SurfaceComposerClient::getStaticDisplayInfo(const sp<IBinder>& display, 
    return statusTFromBinderStatus(status);

}



status_t SurfaceComposerClient::getDynamicDisplayInfo(const sp<IBinder>& display,

                                                      ui::DynamicDisplayInfo* outInfo) {

    gui::DynamicDisplayInfo ginfo;

    binder::Status status =

            ComposerServiceAIDL::getComposerService()->getDynamicDisplayInfo(display, &ginfo);

    if (status.isOk()) {

void SurfaceComposerClient::getDynamicDisplayInfoInternal(gui::DynamicDisplayInfo& ginfo,

                                                          ui::DynamicDisplayInfo*& outInfo) {

    // convert gui::DynamicDisplayInfo to ui::DynamicDisplayInfo

    outInfo->supportedDisplayModes.clear();

    outInfo->supportedDisplayModes.reserve(ginfo.supportedDisplayModes.size());
@@ -2360,13 +2356,36 @@ status_t SurfaceComposerClient::getDynamicDisplayInfo(const sp<IBinder>& display 
    outInfo->gameContentTypeSupported = ginfo.gameContentTypeSupported;

    outInfo->preferredBootDisplayMode = ginfo.preferredBootDisplayMode;

}



status_t SurfaceComposerClient::getDynamicDisplayInfoFromId(int64_t displayId,

                                                            ui::DynamicDisplayInfo* outInfo) {

    gui::DynamicDisplayInfo ginfo;

    binder::Status status =

            ComposerServiceAIDL::getComposerService()->getDynamicDisplayInfoFromId(displayId,

                                                                                   &ginfo);

    if (status.isOk()) {

        getDynamicDisplayInfoInternal(ginfo, outInfo);

    }

    return statusTFromBinderStatus(status);

}



status_t SurfaceComposerClient::getDynamicDisplayInfoFromToken(const sp<IBinder>& display,

                                                               ui::DynamicDisplayInfo* outInfo) {

    gui::DynamicDisplayInfo ginfo;

    binder::Status status =

            ComposerServiceAIDL::getComposerService()->getDynamicDisplayInfoFromToken(display,

                                                                                      &ginfo);

    if (status.isOk()) {

        getDynamicDisplayInfoInternal(ginfo, outInfo);

    }

    return statusTFromBinderStatus(status);

}



status_t SurfaceComposerClient::getActiveDisplayMode(const sp<IBinder>& display,

                                                     ui::DisplayMode* mode) {

    ui::DynamicDisplayInfo info;

    status_t result = getDynamicDisplayInfo(display, &info);



    status_t result = getDynamicDisplayInfoFromToken(display, &info);

    if (result != NO_ERROR) {

        return result;

    }

libs/gui/aidl/android/gui/ISurfaceComposer.aidl

+4 −2
Original line number Diff line number Diff line
@@ -126,12 +126,14 @@ interface ISurfaceComposer { 
    /**

     * Gets immutable information about given physical display.

     */

    StaticDisplayInfo getStaticDisplayInfo(IBinder display);

    StaticDisplayInfo getStaticDisplayInfo(long displayId);



    /**

     * Gets dynamic information about given physical display.

     */

    DynamicDisplayInfo getDynamicDisplayInfo(IBinder display);

    DynamicDisplayInfo getDynamicDisplayInfoFromId(long displayId);



    DynamicDisplayInfo getDynamicDisplayInfoFromToken(IBinder display);



    DisplayPrimaries getDisplayNativePrimaries(IBinder display);

libs/gui/fuzzer/libgui_fuzzer_utils.h

+4 −2
Original line number Diff line number Diff line
@@ -79,9 +79,11 @@ public: 
                (override));

    MOCK_METHOD(binder::Status, getDisplayState, (const sp<IBinder>&, gui::DisplayState*),

                (override));

    MOCK_METHOD(binder::Status, getStaticDisplayInfo, (const sp<IBinder>&, gui::StaticDisplayInfo*),

    MOCK_METHOD(binder::Status, getStaticDisplayInfo, (int64_t, gui::StaticDisplayInfo*),

                (override));

    MOCK_METHOD(binder::Status, getDynamicDisplayInfo,

    MOCK_METHOD(binder::Status, getDynamicDisplayInfoFromId, (int64_t, gui::DynamicDisplayInfo*),

                (override));

    MOCK_METHOD(binder::Status, getDynamicDisplayInfoFromToken,

                (const sp<IBinder>&, gui::DynamicDisplayInfo*), (override));

    MOCK_METHOD(binder::Status, getDisplayNativePrimaries,

                (const sp<IBinder>&, gui::DisplayPrimaries*), (override));

libs/gui/include/gui/LayerState.h

+1 −0
Original line number Diff line number Diff line
@@ -359,6 +359,7 @@ struct DisplayState { 


    DisplayState();

    void merge(const DisplayState& other);

    void sanitize(int32_t permissions);



    uint32_t what = 0;

    uint32_t flags = 0;