Do not allow invalid quoted expression to leak

Closes #2499

Author: José Valim

lib/elixir/lib/kernel.ex

@@ -2013,7 +2013,7 @@ defmodule Kernel do
   defp do_at([arg], name, function?, env) do
     case function? do
       true ->
-        raise ArgumentError, "cannot dynamically set attribute @#{name} inside function"
+        raise ArgumentError, "cannot set attribute @#{name} inside function/macro"
       false ->
         case name do
           :behavior ->
@@ -2034,7 +2034,14 @@ defmodule Kernel do
     case function? do
       true ->
         attr = Module.get_attribute(env.module, name, stack)
-        :erlang.element(1, :elixir_quote.escape(attr, false))
+        try do
+          :elixir_quote.escape(attr, false)
+        rescue
+          e in [ArgumentError] ->
+            raise ArgumentError, "cannot inject attribute @#{name} into function/macro because " <> Exception.message(e)
+        else
+          {val, _} -> val
+        end
       false ->
         escaped = case stack do
           [] -> []
@@ -2901,31 +2908,35 @@ defmodule Kernel do
   structure is public should use `@type`.
   """
   defmacro defstruct(fields) do
-    fields =
-      quote bind_quoted: [fields: fields] do
-        fields = :lists.map(fn
-          {key, _} = pair when is_atom(key) -> pair
-          key when is_atom(key) -> {key, nil}
-          other -> raise ArgumentError, "struct field names must be atoms, got: #{inspect other}"
-        end, fields)
-
-        @struct :maps.put(:__struct__, __MODULE__, :maps.from_list(fields))
-
-        case Module.get_attribute(__MODULE__, :derive) do
-          [] ->
-            :ok
-          derive ->
-            Protocol.__derive__(derive, __MODULE__, __ENV__)
-        end
+    quote bind_quoted: [fields: fields] do
+      fields = :lists.map(fn
+        {key, val} when is_atom(key) ->
+          try do
+            Macro.escape(val)
+          rescue
+            e in [ArgumentError] ->
+              raise ArgumentError, "invalid value for struct field #{key}, " <> Exception.message(e)
+          else
+            _ -> {key, val}
+          end
+        key when is_atom(key) ->
+          {key, nil}
+        other ->
+          raise ArgumentError, "struct field names must be atoms, got: #{inspect other}"
+      end, fields)
 
-        @spec __struct__() :: %__MODULE__{}
-        def __struct__() do
-          @struct
-        end
+      @struct :maps.put(:__struct__, __MODULE__, :maps.from_list(fields))
+
+      case Module.get_attribute(__MODULE__, :derive) do
+        [] -> :ok
+        derive -> Protocol.__derive__(derive, __MODULE__, __ENV__)
+      end
+
+      @spec __struct__() :: %__MODULE__{}
+      def __struct__() do
+        @struct
       end
 
-    quote do
-      unquote(fields)
       fields
     end
   end

lib/elixir/lib/record.ex

@@ -160,7 +160,7 @@ defmodule Record do
   defmacro defrecord(name, tag \\ nil, kv) do
     quote bind_quoted: [name: name, tag: tag, kv: kv] do
       tag = tag || name
-      fields = Macro.escape Record.__fields__(:defrecord, kv)
+      fields = Record.__fields__(:defrecord, kv)
 
       defmacro(unquote(name)(args \\ [])) do
         Record.__access__(unquote(tag), unquote(fields), args, __CALLER__)
@@ -178,7 +178,7 @@ defmodule Record do
   defmacro defrecordp(name, tag \\ nil, kv) do
     quote bind_quoted: [name: name, tag: tag, kv: kv] do
       tag = tag || name
-      fields = Macro.escape Record.__fields__(:defrecordp, kv)
+      fields = Record.__fields__(:defrecordp, kv)
 
       defmacrop(unquote(name)(args \\ [])) do
         Record.__access__(unquote(tag), unquote(fields), args, __CALLER__)
@@ -194,9 +194,19 @@ defmodule Record do
   @doc false
   def __fields__(type, fields) do
     :lists.map(fn
-      { key, _ } = pair when is_atom(key) -> pair
-      key when is_atom(key) -> { key, nil }
-      other -> raise ArgumentError, "#{type} fields must be atoms, got: #{inspect other}"
+      {key, val} when is_atom(key) ->
+        try do
+          Macro.escape(val)
+        rescue
+          e in [ArgumentError] ->
+            raise ArgumentError, "invalid value for record field #{key}, " <> Exception.message(e)
+        else
+          val -> {key, val}
+        end
+      key when is_atom(key) ->
+        {key, nil}
+      other ->
+        raise ArgumentError, "#{type} fields must be atoms, got: #{inspect other}"
     end, fields)
   end
 

lib/elixir/src/elixir_quote.erl

@@ -242,7 +242,7 @@ do_quote({Left, Right}, Q, E) ->
   {TRight, RQ} = do_quote(Right, LQ, E),
   {{TLeft, TRight}, RQ};
 
-do_quote(BitString, #elixir_quote{escape=true} = Q, E) when is_bitstring(BitString) ->
+do_quote(BitString, #elixir_quote{escape=true} = Q, _) when is_bitstring(BitString) ->
   case bit_size(BitString) rem 8 of
     0 ->
       {BitString, Q};
@@ -260,7 +260,7 @@ do_quote(Tuple, #elixir_quote{escape=true} = Q, E) when is_tuple(Tuple) ->
   {{'{}', [], TT}, TQ};
 
 do_quote(List, #elixir_quote{escape=true} = Q, E) when is_list(List) ->
-  % The improper case is pretty inefficient, but improper lists are are.
+  %% The improper case is a bit inefficient, but improper lists are rare.
   case reverse_improper(List) of
     {L}       -> do_splice(L, Q, E);
     {L, R}    ->
@@ -269,14 +269,34 @@ do_quote(List, #elixir_quote{escape=true} = Q, E) when is_list(List) ->
       {update_last(TL, fun(X) -> {'|', [], [X, TR]} end), QR}
   end;
 
+do_quote(Other, #elixir_quote{escape=true} = Q, _)
+    when is_number(Other); is_pid(Other); is_atom(Other) ->
+  {Other, Q};
+
+do_quote(Fun, #elixir_quote{escape=true} = Q, _) when is_function(Fun) ->
+  case (erlang:fun_info(Fun, env) == {env, []}) andalso
+       (erlang:fun_info(Fun, type) == {type, external}) of
+    true  -> {Fun, Q};
+    false -> bad_escape(Fun)
+  end;
+
+do_quote(Other, #elixir_quote{escape=true}, _) ->
+  bad_escape(Other);
+
 do_quote(List, Q, E) when is_list(List) ->
-    do_splice(lists:reverse(List), Q, E);
+  do_splice(lists:reverse(List), Q, E);
 
 do_quote(Other, Q, _) ->
   {Other, Q}.
 
 %% Quote helpers
 
+bad_escape(Arg) ->
+  Msg = <<"cannot escape ", ('Elixir.Kernel':inspect(Arg, []))/binary, ". ",
+          "The supported values are: lists, tuples, maps, atoms, numbers, bitstrings, ",
+          "pids and remote functions in the format &Mod.fun/arity">>,
+  error('Elixir.ArgumentError':exception([{message,Msg}])).
+
 do_quote_call(Left, Meta, Expr, Args, Q, E) ->
   All  = [meta(Meta, Q), Left, {unquote, Meta, [Expr]}, Args,
           Q#elixir_quote.context],

lib/elixir/test/elixir/kernel/errors_test.exs

@@ -324,6 +324,16 @@ defmodule Kernel.ErrorsTest do
       'casea foo, do: @hello :world'
   end
 
+  test :invalid_attribute do
+    msg = ~r"cannot inject attribute @foo into function/macro because cannot escape "
+    assert_raise ArgumentError, msg, fn ->
+      defmodule Foo do
+        @foo fn -> end
+        def bar, do: @foo
+      end
+    end
+  end
+
   test :invalid_fn_args do
     assert_compile_fail TokenMissingError,
       "nofile:1: missing terminator: end (for \"fn\" starting at line 1)",