!263 IPC: modify FindOrNewObject prototype to avoid UAF problems caused by mixing raws ptr and sptr

Merge pull request !263 from liubb_0516/OpenHarmony-3.1-Release

interfaces/innerkits/ipc_core/include/iremote_object.h

@@ -66,7 +66,7 @@ public:
 
     virtual bool Marshalling(Parcel &parcel) const override;
 
-    static IRemoteObject *Unmarshalling(Parcel &parcel);
+    static sptr<IRemoteObject> Unmarshalling(Parcel &parcel);
 
     static bool Marshalling(Parcel &parcel, const sptr<IRemoteObject> &object);
 

ipc/native/src/core/include/ipc_process_skeleton.h

@@ -94,7 +94,7 @@ public:
 
     std::u16string MakeHandleDescriptor(int handle);
 
-    IRemoteObject *FindOrNewObject(int handle);
+    sptr<IRemoteObject> FindOrNewObject(int handle);
     bool IsContainsObject(IRemoteObject *object);
     IRemoteObject *QueryObject(const std::u16string &descriptor);
     IRemoteObject *QueryObjectInner(const std::u16string &descriptor);

ipc/native/src/core/source/ipc_process_skeleton.cpp

@@ -121,14 +121,14 @@ std::u16string IPCProcessSkeleton::MakeHandleDescriptor(int handle)
     return to_utf16(descriptor);
 }
 
-IRemoteObject *IPCProcessSkeleton::FindOrNewObject(int handle)
+sptr<IRemoteObject> IPCProcessSkeleton::FindOrNewObject(int handle)
 {
-    IRemoteObject *remoteObject = nullptr;
+    sptr<IRemoteObject> result = nullptr;
     std::u16string descriptor = MakeHandleDescriptor(handle);
     {
         std::lock_guard<std::recursive_mutex> lock(mutex_);
 
-        remoteObject = QueryObjectInner(descriptor);
+        IRemoteObject *remoteObject = QueryObjectInner(descriptor);
         if (remoteObject == nullptr) {
             if (handle == REGISTRY_HANDLE) {
                 IRemoteInvoker *invoker = IPCThreadSkeleton::GetRemoteInvoker(IRemoteObject::IF_PROT_DEFAULT);
@@ -146,21 +146,18 @@ IRemoteObject *IPCProcessSkeleton::FindOrNewObject(int handle)
             if (proxy == nullptr) {
                 return nullptr;
             }
-            proxy->AttemptAcquire(this); // AttemptAcquire always returns true as life time is extended
             remoteObject = reinterpret_cast<IRemoteObject *>(proxy);
             if (!AttachObjectInner(remoteObject)) {
-                DBINDER_LOGE("attach object failed");
                 delete proxy;
                 return nullptr;
             }
-        } else {
-            remoteObject->AttemptAcquire(this);
         }
+        result = remoteObject;
     }
 
-    IPCObjectProxy *remoteProxy = reinterpret_cast<IPCObjectProxy *>(remoteObject);
+    sptr<IPCObjectProxy> remoteProxy = reinterpret_cast<IPCObjectProxy *>(result.GetRefPtr());
     remoteProxy->WaitForInit();
-    return remoteObject;
+    return result;
 }
 
 bool IPCProcessSkeleton::SetMaxWorkThread(int maxThreadNum)

ipc/native/src/core/source/iremote_object.cpp

@@ -49,7 +49,7 @@ bool IRemoteObject::Marshalling(Parcel &parcel, const sptr<IRemoteObject> &objec
     return false;
 }
 
-IRemoteObject *IRemoteObject::Unmarshalling(Parcel &parcel)
+sptr<IRemoteObject> IRemoteObject::Unmarshalling(Parcel &parcel)
 {
     IRemoteInvoker *invoker = IPCThreadSkeleton::GetRemoteInvoker(IRemoteObject::IF_PROT_DEFAULT);
     if (invoker != nullptr) {

ipc/native/src/mock/include/binder_invoker.h

@@ -70,7 +70,7 @@ public:
 
     bool FlattenObject(Parcel &parcel, const IRemoteObject *object) const override;
 
-    IRemoteObject *UnflattenObject(Parcel &parcel) override;
+    sptr<IRemoteObject> UnflattenObject(Parcel &parcel) override;
 
     int ReadFileDescriptor(Parcel &parcel) override;
 

ipc/native/src/mock/include/dbinder_databus_invoker.h

@@ -41,7 +41,7 @@ public:
     void JoinProcessThread(bool initiative) override;
     void StopWorkThread() override;
     bool FlattenObject(Parcel &parcel, const IRemoteObject *object) const override;
-    IRemoteObject *UnflattenObject(Parcel &parcel) override;
+    sptr<IRemoteObject> UnflattenObject(Parcel &parcel) override;
     int ReadFileDescriptor(Parcel &parcel) override;
     bool WriteFileDescriptor(Parcel &parcel, int fd, bool takeOwnership) override;
     pid_t GetCallerPid() const override;

ipc/native/src/mock/include/iremote_invoker.h

@@ -82,7 +82,7 @@ public:
 
     virtual bool FlattenObject(Parcel &parcel, const IRemoteObject *object) const = 0;
 
-    virtual IRemoteObject *UnflattenObject(Parcel &parcel) = 0;
+    virtual sptr<IRemoteObject> UnflattenObject(Parcel &parcel) = 0;
 
     virtual int ReadFileDescriptor(Parcel &parcel) = 0;
 

ipc/native/src/mock/source/binder_invoker.cpp

@@ -889,7 +889,7 @@ bool BinderInvoker::FlattenObject(Parcel &parcel, const IRemoteObject *object) c
     return status;
 }
 
-IRemoteObject *BinderInvoker::UnflattenObject(Parcel &parcel)
+sptr<IRemoteObject> BinderInvoker::UnflattenObject(Parcel &parcel)
 {
     const uint8_t *buffer = parcel.ReadBuffer(sizeof(flat_binder_object));
     if (buffer == nullptr) {
@@ -902,7 +902,7 @@ IRemoteObject *BinderInvoker::UnflattenObject(Parcel &parcel)
         return nullptr;
     }
 
-    IRemoteObject *remoteObject = nullptr;
+    sptr<IRemoteObject> remoteObject = nullptr;
     auto *flat = reinterpret_cast<const flat_binder_object *>(buffer);
     switch (flat->hdr.type) {
         case BINDER_TYPE_BINDER: {

ipc/native/src/mock/source/dbinder_databus_invoker.cpp

@@ -78,7 +78,7 @@ std::shared_ptr<DBinderSessionObject> DBinderDatabusInvoker::NewSessionOfBinderP
         return nullptr;
     }
 
-    IPCObjectProxy *ipcProxy = reinterpret_cast<IPCObjectProxy *>(current->FindOrNewObject(handle));
+    sptr<IPCObjectProxy> ipcProxy = reinterpret_cast<IPCObjectProxy *>(current->FindOrNewObject(handle).GetRefPtr());
     if (ipcProxy == nullptr) {
         DBINDER_LOGE("attempt to send a invalid handle = %u", handle);
         return nullptr;
@@ -480,7 +480,7 @@ bool DBinderDatabusInvoker::FlattenObject(Parcel &parcel, const IRemoteObject *o
     return true;
 }
 
-IRemoteObject *DBinderDatabusInvoker::UnflattenObject(Parcel &parcel)
+sptr<IRemoteObject> DBinderDatabusInvoker::UnflattenObject(Parcel &parcel)
 {
     return nullptr;
 }