bugfix: repro and fix for issue #4267 (reject too long principals in …

…Principal.fromBlob and actor literals) (#4268)

Fixes issues #4267:

  * BREAKING CHANGE (Minor): values of type `Principal` are now constrained to contain
    at most 29 bytes, matching the IC's notion of principal (#4268).

    In particular:

    * An actor `import` will be statically rejected if the binary representation of the (aliased) textually encoded
    principal contains strictly more than 29 bytes.

    * `Principal.fromBlob(b)` will trap if `b` contains strictly more than 29 bytes.

    *  The actor literal, `actor <exp>`, will trap if the binary representation of
    of the textually encoded principal `<exp>` contains strictly more than 29 bytes.

Changelog.md

@@ -1,5 +1,21 @@
 # Motoko compiler changelog
 
+* motoko (`moc`)
+
+  * BREAKING CHANGE (Minor): values of type `Principal` are now constrained to contain
+    at most 29 bytes, matching the IC's notion of principal (#4268).
+
+    In particular:
+
+    * An actor `import` will be statically rejected if the binary representation of the (aliased) textually encoded
+    principal contains strictly more than 29 bytes.
+
+    * `Principal.fromBlob(b)` will trap if `b` contains strictly more than 29 bytes.
+
+    *  The actor literal, `actor <exp>`, will trap if the binary representation of
+    of the textually encoded principal `<exp>` contains strictly more than 29 bytes.
+
+
 ## 0.10.1 (2023-10-16)
 
 * motoko (`moc`)

src/codegen/compile.ml

@@ -10985,7 +10985,16 @@ and compile_prim_invocation (env : E.t) ae p es at =
 
   (* Actor ids are blobs in the RTS *)
   | ActorOfIdBlob _, [e] ->
-    compile_exp env ae e
+    SR.Vanilla,
+    let (set_blob, get_blob) = new_local env "blob" in
+    compile_exp_vanilla env ae e ^^
+    set_blob ^^
+    get_blob ^^
+    Blob.len env ^^
+    compile_unboxed_const 29l ^^
+    G.i (Compare (Wasm.Values.I32 I32Op.LeU)) ^^
+    E.else_trap_with env "blob too long for actor principal" ^^
+    get_blob
 
   | SelfRef _, [] ->
     SR.Vanilla, IC.get_self_reference env

src/ir_interpreter/interpret_ir.ml

@@ -412,7 +412,10 @@ and interpret_exp_mut env exp (k : V.value V.cont) =
       | CastPrim _, [v1] ->
         k v1
       | ActorOfIdBlob t, [v1] ->
-        k v1
+        if String.length (V.as_blob v1) > 29 then
+          trap exp.at "blob too long for actor principal"
+        else
+          k v1
       | DecodeUtf8, [v1] ->
         let s = V.as_blob v1 in
         begin match Lib.Utf8.decode s with

src/mo_interpreter/interpret.ml

@@ -436,7 +436,11 @@ and interpret_exp_mut env exp (k : V.value V.cont) =
       let url_text = V.as_text v1 in
       match Ic.Url.decode_principal url_text with
       (* create placeholder functions (see #3683) *)
-      | Ok bytes -> k (V.Blob bytes)
+      | Ok bytes ->
+        if String.length bytes > 29 then
+          trap exp.at "blob too long for actor principal"
+        else
+          k (V.Blob bytes)
       | Error e -> trap exp.at "could not parse %S as an actor reference: %s"  (V.as_text v1) e
     )
   | UnE (ot, op, exp1) ->

src/pipeline/resolve_import.ml

@@ -170,7 +170,7 @@ let resolve_import_string msgs base actor_idl_path aliases packages imported (f,
     | Some actor_base ->
       let full_path = in_base actor_base (Url.idl_basename_of_blob bytes) in
       add_idl_import msgs imported ri_ref at full_path bytes
-    in
+  in
   match Url.parse f with
   | Ok (Url.Relative path) ->
     (* TODO support importing local .did file *)
@@ -180,7 +180,11 @@ let resolve_import_string msgs base actor_idl_path aliases packages imported (f,
     | Some pkg_path -> add_lib_import msgs imported ri_ref at (in_base pkg_path path)
     | None -> err_package_not_defined msgs at pkg
     end
-  | Ok (Url.Ic bytes) -> resolve_ic bytes
+  | Ok (Url.Ic bytes) ->
+     if String.length bytes > 29 then
+       err_unrecognized_url msgs at f "Principal too long"
+     else
+     resolve_ic bytes
   | Ok (Url.IcAlias alias) ->
     begin match M.find_opt alias aliases with
     | Some bytes -> resolve_ic bytes
@@ -202,7 +206,10 @@ let resolve_package_url (msgs:Diag.msg_store) (pname:string) (f:url) : filepath
 (* Resolve the argument to --actor-alias. Check eagerly for well-formedness *)
 let resolve_alias_principal (msgs:Diag.msg_store) (alias:string) (f:string) : blob =
   match Url.decode_principal f with
-  | Ok bytes -> bytes
+  | Ok bytes ->
+     if String.length bytes > 29 then
+       (err_unrecognized_alias msgs alias f "Principal too long"; "")
+     else bytes
   | Error msg -> err_unrecognized_alias msgs alias f msg; ""
 
 

src/prelude/prim.mo

@@ -308,7 +308,12 @@ func time() : Nat64 = (prim "time" : () -> Nat64)();
 // Principal
 
 func blobOfPrincipal(id : Principal) : Blob = (prim "cast" : Principal -> Blob) id;
-func principalOfBlob(act : Blob) : Principal = (prim "cast" : Blob -> Principal) act;
+func principalOfBlob(act : Blob) : Principal {
+  if (act.size() > 29) {
+    trap("blob too long for principal");
+  };
+  (prim "cast" : Blob -> Principal) act;
+};
 
 func principalOfActor(act : actor {}) : Principal = (prim "cast" : (actor {}) -> Principal) act;
 func isController(p : Principal) : Bool = (prim "is_controller" : Principal -> Bool) p;

test/bench/ok/heap-32.drun-run-opt.ok

@@ -1,5 +1,5 @@
 ingress Completed: Reply: 0x4449444c016c01b3c4b1f204680100010a00000000000000000101
 ingress Completed: Reply: 0x4449444c0000
-debug.print: (50_227, +30_261_252, 620_249_132)
-debug.print: (50_070, +32_992_212, 671_159_920)
+debug.print: (50_227, +30_261_252, 620_249_119)
+debug.print: (50_070, +32_992_212, 671_159_959)
 ingress Completed: Reply: 0x4449444c0000

test/bench/ok/heap-32.drun-run.ok

@@ -1,5 +1,5 @@
 ingress Completed: Reply: 0x4449444c016c01b3c4b1f204680100010a00000000000000000101
 ingress Completed: Reply: 0x4449444c0000
-debug.print: (50_227, +30_261_252, 667_502_658)
-debug.print: (50_070, +32_992_212, 720_277_365)
+debug.print: (50_227, +30_261_252, 667_502_633)
+debug.print: (50_070, +32_992_212, 720_277_440)
 ingress Completed: Reply: 0x4449444c0000

test/run-drun/bad-actor-import.mo

@@ -0,0 +1,5 @@
+//MOC-FLAG --actor-idl bad-actor-import
+import c "ic:yvtf6-waaae-bagba-faydq-qcikb-mga2d-qpcai-reeyu-culbo-gazdi-nryhi";
+actor {
+
+}

test/run-drun/bad-actor-import/yvtf6-waaae-bagba-faydq-qcikb-mga2d-qpcai-reeyu-culbo-gazdi-nryhi.did

@@ -0,0 +1,3 @@
+service a : {
+
+}

test/run-drun/ok/bad-actor-import.tc.ok

@@ -0,0 +1 @@
+bad-actor-import.mo:2.1-2.80: import error [M0006], cannot parse import URL "ic:yvtf6-waaae-bagba-faydq-qcikb-mga2d-qpcai-reeyu-culbo-gazdi-nryhi": Principal too long

test/run-drun/ok/bad-actor-import.tc.ret.ok

@@ -0,0 +1 @@
+Return code 1

test/run/bad-principal-from-blob.mo

@@ -0,0 +1,8 @@
+import Prim "mo:⛔";
+
+// construct a 30 byte principal (illegal)
+let a = Prim.Array_tabulate<Nat8>(30, func i {Prim.natToNat8(i)});
+let b = Prim.arrayToBlob(a);
+let p = Prim.principalOfBlob(b); // should trap!
+let t = debug_show(p);
+Prim.debugPrint(t);

test/run/bad-principal-from-text.mo

@@ -0,0 +1,7 @@
+import Prim "mo:⛔";
+
+// illegal textual principal (30 bytes decoded)
+let t = "yvtf6-waaae-bagba-faydq-qcikb-mga2d-qpcai-reeyu-culbo-gazdi-nryhi";
+// construct an actor from an illegal 30 byte principal
+let bad = Prim.principalOfActor(actor (t)); // should trap
+Prim.debugPrint(debug_show(bad));

test/run/ok/bad-principal-from-blob.run-ir.ok

@@ -0,0 +1 @@
+prim:___: execution error, explicit trap: blob too long for principal

test/run/ok/bad-principal-from-blob.run-low.ok

@@ -0,0 +1 @@
+prim:___: execution error, explicit trap: blob too long for principal

test/run/ok/bad-principal-from-blob.run.ok

@@ -0,0 +1 @@
+prim:___: execution error, explicit trap: blob too long for principal

test/run/ok/bad-principal-from-blob.run.ret.ok

@@ -0,0 +1 @@
+Return code 1

test/run/ok/bad-principal-from-blob.wasm-run.ok

@@ -0,0 +1,10 @@
+blob too long for principal
+Error: failed to run main module `_out/bad-principal-from-blob.wasm`
+
+Caused by:
+    0: failed to invoke command default
+    1: error while executing at wasm backtrace:
+         0: principalOfBlob
+         1: init
+         2: _start
+    2: wasm trap: unreachable

test/run/ok/bad-principal-from-blob.wasm-run.ret.ok

@@ -0,0 +1 @@
+Return code 134

test/run/ok/bad-principal-from-text.run-ir.ok

@@ -0,0 +1 @@
+bad-principal-from-text.mo:6.33-6.42: execution error, blob too long for actor principal

test/run/ok/bad-principal-from-text.run-low.ok

@@ -0,0 +1 @@
+bad-principal-from-text.mo:6.33-6.42: execution error, blob too long for actor principal

test/run/ok/bad-principal-from-text.run.ok

@@ -0,0 +1 @@
+bad-principal-from-text.mo:6.33-6.42: execution error, blob too long for actor principal

test/run/ok/bad-principal-from-text.run.ret.ok

@@ -0,0 +1 @@
+Return code 1

test/run/ok/bad-principal-from-text.wasm-run.ok

@@ -0,0 +1,9 @@
+blob too long for actor principal
+Error: failed to run main module `_out/bad-principal-from-text.wasm`
+
+Caused by:
+    0: failed to invoke command default
+    1: error while executing at wasm backtrace:
+         0: init
+         1: _start
+    2: wasm trap: unreachable

test/run/ok/bad-principal-from-text.wasm-run.ret.ok

@@ -0,0 +1 @@
+Return code 134