Bug 1610017 - Actually taint data when tainted is specified for IPC methods r=nika

Differential Revision: https://phabricator.services.mozilla.com/D60532

--HG--
extra : moz-landing-system : lando

ipc/glue/TaintingIPCUtils.h

@@ -0,0 +1,41 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef mozilla_ipc_Tainting_h
+#define mozilla_ipc_Tainting_h
+
+#include "mozilla/Tainting.h"
+
+#include "base/basictypes.h"
+#include "base/process.h"
+
+#include "mozilla/ipc/IPDLParamTraits.h"
+
+namespace mozilla {
+namespace ipc {
+
+template <typename T>
+struct IPDLParamTraits<mozilla::Tainted<T>> {
+  static void Write(IPC::Message* aMsg, IProtocol* aActor,
+                    const mozilla::Tainted<T>& aParam) {
+    WriteIPDLParam(aMsg, aActor, aParam.mValue);
+  }
+
+  static void Write(IPC::Message* aMsg, IProtocol* aActor,
+                    mozilla::Tainted<T>&& aParam) {
+    WriteIPDLParam(aMsg, aActor, std::move(aParam.mValue));
+  }
+
+  static bool Read(const IPC::Message* aMsg, PickleIterator* aIter,
+                   IProtocol* aActor, mozilla::Tainted<T>* aResult) {
+    return ReadIPDLParam(aMsg, aIter, aActor, &(aResult->mValue));
+  }
+};
+
+}  // namespace ipc
+}  // namespace mozilla
+
+#endif  // ifndef mozilla_ipc_Tainting_h

ipc/glue/moz.build

@@ -49,6 +49,7 @@ EXPORTS.mozilla.ipc += [
     'SharedMemory.h',
     'SharedMemoryBasic.h',
     'Shmem.h',
+    'TaintingIPCUtils.h',
     'TaskFactory.h',
     'Transport.h',
     'TransportSecurityInfoUtils.h',

ipc/ipdl/ipdl/builtin.py

@@ -51,6 +51,7 @@ HeaderIncludes = (
     'nsString.h',
     'nsTArray.h',
     'mozilla/ipc/ProtocolUtils.h',
+    'mozilla/ipc/TaintingIPCUtils.h',
     'nsTHashtable.h',
     'mozilla/OperatorNewExtensions.h',
     'mozilla/UniquePtr.h',

ipc/ipdl/ipdl/lower.py

@@ -1093,11 +1093,16 @@ class MessageDecl(ipdl.ast.MessageDecl):
         return self.params[0]
 
     def makeCxxParams(self, paramsems='in', returnsems='out',
-                      side=None, implicit=True):
+                      side=None, implicit=True, direction=None):
         """Return a list of C++ decls per the spec'd configuration.
 |params| and |returns| is the C++ semantics of those: 'in', 'out', or None."""
 
         def makeDecl(d, sems):
+            if self.decl.type.tainted and direction == 'recv':
+                # Tainted types are passed by-value, allowing the receiver to move them if desired.
+                assert sems != 'out'
+                return Decl(Type('Tainted', T=d.bareType(side)), d.name)
+
             if sems == 'in':
                 return Decl(d.inType(side), d.name)
             elif sems == 'move':
@@ -3266,6 +3271,7 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor):
             #endif  // DEBUG
 
             #include "base/id_map.h"
+            #include "mozilla/Tainting.h"
             #include "mozilla/ipc/MessageChannel.h"
             #include "mozilla/ipc/ProtocolUtils.h"
             ''')
@@ -3364,7 +3370,7 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor):
                 recvDecl = MethodDecl(
                     md.recvMethod(),
                     params=md.makeCxxParams(paramsems='move', returnsems=returnsems,
-                                            side=self.side, implicit=implicit),
+                                            side=self.side, implicit=implicit, direction='recv'),
                     ret=Type('mozilla::ipc::IPCResult'),
                     methodspec=MethodSpec.VIRTUAL)
 
@@ -3405,7 +3411,7 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor):
 
                 self.cls.addstmt(StmtDecl(MethodDecl(
                     _allocMethod(managed, self.side),
-                    params=md.makeCxxParams(side=self.side, implicit=False),
+                    params=md.makeCxxParams(side=self.side, implicit=False, direction='recv'),
                     ret=actortype, methodspec=MethodSpec.PURE)))
 
             # add the Dealloc interface for all managed non-refcounted actors,
@@ -4657,18 +4663,26 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor):
                                               actor=ExprVar.THIS)]
             start = 1
 
+        decls.extend([StmtDecl(Decl(
+                                   (Type('Tainted', T=p.bareType(side))
+                                    if md.decl.type.tainted else
+                                    p.bareType(side)),
+                                   p.var().name))
+                      for p in md.params[start:]])
+        reads.extend([_ParamTraits.checkedRead(p.ipdltype,
+                                               ExprAddrOf(p.var()),
+                                               msgexpr, ExprAddrOf(itervar),
+                                               errfn, "'%s'" % p.ipdltype.name(),
+                                               sentinelKey=p.name, errfnSentinel=errfnSent,
+                                               actor=ExprVar.THIS)
+                      for p in md.params[start:]])
+
         stmts.extend((
             [StmtDecl(Decl(_iterType(ptr=False), self.itervar.name),
                       initargs=[msgvar])]
-            + decls + [StmtDecl(Decl(p.bareType(side), p.var().name))
-                       for p in md.params[start:]]
+            + decls
             + [Whitespace.NL]
-            + reads + [_ParamTraits.checkedRead(p.ipdltype, ExprAddrOf(p.var()),
-                                                msgexpr, ExprAddrOf(itervar),
-                                                errfn, "'%s'" % p.ipdltype.name(),
-                                                sentinelKey=p.name, errfnSentinel=errfnSent,
-                                                actor=ExprVar.THIS)
-                       for p in md.params[start:]]
+            + reads
             + [self.endRead(msgvar, itervar)]))
 
         return stmts
@@ -4904,7 +4918,7 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor):
         decl = MethodDecl(
             md.sendMethod(),
             params=md.makeCxxParams(paramsems, returnsems=returnsems,
-                                    side=self.side, implicit=implicit),
+                                    side=self.side, implicit=implicit, direction='send'),
             warn_unused=((self.side == 'parent' and returnsems != 'callback') or
                          (md.decl.type.isCtor() and not md.decl.type.isAsync())),
             ret=rettype)

mfbt/Tainting.h

@@ -16,6 +16,14 @@
 
 namespace mozilla {
 
+template <typename T>
+class Tainted;
+
+namespace ipc {
+template <typename>
+struct IPDLParamTraits;
+}
+
 /*
  * The Tainted<> class allows data to be wrapped and considered 'tainted'; which
  * requires explicit validation of the data before it can be used for
@@ -50,10 +58,15 @@ class Tainted {
   T mValue;
 
  public:
+  explicit Tainted() = default;
+
   template <typename U>
   explicit Tainted(U&& aValue) : mValue(std::forward<U>(aValue)) {}
 
   T& Coerce() { return this->mValue; }
+  const T& Coerce() const { return this->mValue; }
+
+  friend struct mozilla::ipc::IPDLParamTraits<Tainted<T>>;
 };
 
 // ================================================
@@ -115,11 +128,16 @@ class Tainted {
 // We use the same variable name in the nested scope, shadowing the outer
 // scope - this allows the user to write the same variable name in the
 // macro's condition without using a magic name like 'value'.
+//
+// We mark it MOZ_MAYBE_UNUSED because sometimes the condition doesn't
+// make use of tainted_value, which causes an unused variable warning.
+// That will only happen when we are bypssing validation, which is a
+// future ergonomic we will iterate on.
 #define MOZ_VALIDATE_AND_GET_HELPER3(tainted_value, condition, \
                                      assertionstring)          \
   [&tainted_value]() {                                         \
     auto& tmp = tainted_value.Coerce();                        \
-    auto& tainted_value = tmp;                                 \
+    auto& MOZ_MAYBE_UNUSED tainted_value = tmp;                \
     MOZ_RELEASE_ASSERT((condition), assertionstring);          \
     return tmp;                                                \
   }()
@@ -139,11 +157,11 @@ class Tainted {
 // This construct uses a lambda expression to create a scope and test the
 // condition, returning true or false.
 // We use the same variable-shadowing trick.
-#define MOZ_IS_VALID(tainted_value, condition) \
-  [&tainted_value]() {                         \
-    auto& tmp = tainted_value.Coerce();        \
-    auto& tainted_value = tmp;                 \
-    return (condition);                        \
+#define MOZ_IS_VALID(tainted_value, condition)  \
+  [&tainted_value]() {                          \
+    auto& tmp = tainted_value.Coerce();         \
+    auto& MOZ_MAYBE_UNUSED tainted_value = tmp; \
+    return (condition);                         \
   }()
 
 // Allows us to test validity and returning a default value if the value is