Only allow system and graphics to create secure displays
Previously, we allowed any process that had the permission
ACCESS_SURFACE_FLINGER to create a display, either secure or
not secure. The shell process needs this permission to create
a display for screen recording. However, we just shouldn't allow
any process to create a secure display since that would allow
them to render secure content. Instead, only allow system
and graphics to create secure displays.
Fixes: 154721930
Test: Modified screenrecord to create secure display, which fails
Test: SurfaceFlinger_test
Test: SurfaceInterceptorTest
Test: DisplayTransactionTest
Change-Id: Ib3c5b6c8abd41f3f6fc6a71273cb2a17bfdba959
diff --git a/libs/gui/ISurfaceComposer.cpp b/libs/gui/ISurfaceComposer.cpp
index 0ac493d..c7c75da 100644
--- a/libs/gui/ISurfaceComposer.cpp
+++ b/libs/gui/ISurfaceComposer.cpp
@@ -244,10 +244,25 @@
{
Parcel data, reply;
data.writeInterfaceToken(ISurfaceComposer::getInterfaceDescriptor());
- data.writeString8(displayName);
- data.writeInt32(secure ? 1 : 0);
- remote()->transact(BnSurfaceComposer::CREATE_DISPLAY, data, &reply);
- return reply.readStrongBinder();
+ status_t status = data.writeString8(displayName);
+ if (status) {
+ return nullptr;
+ }
+ status = data.writeBool(secure);
+ if (status) {
+ return nullptr;
+ }
+
+ status = remote()->transact(BnSurfaceComposer::CREATE_DISPLAY, data, &reply);
+ if (status) {
+ return nullptr;
+ }
+ sp<IBinder> display;
+ status = reply.readNullableStrongBinder(&display);
+ if (status) {
+ return nullptr;
+ }
+ return display;
}
virtual void destroyDisplay(const sp<IBinder>& display)
@@ -1295,10 +1310,12 @@
}
case CREATE_DISPLAY: {
CHECK_INTERFACE(ISurfaceComposer, data, reply);
- String8 displayName = data.readString8();
- bool secure = bool(data.readInt32());
- sp<IBinder> display(createDisplay(displayName, secure));
- reply->writeStrongBinder(display);
+ String8 displayName;
+ SAFE_PARCEL(data.readString8, &displayName);
+ bool secure = false;
+ SAFE_PARCEL(data.readBool, &secure);
+ sp<IBinder> display = createDisplay(displayName, secure);
+ SAFE_PARCEL(reply->writeStrongBinder, display);
return NO_ERROR;
}
case DESTROY_DISPLAY: {