Roslyn: Symbol API exposes data that can be mutated by the caller

[CyrusNajmabadi]

Found while working on: https://github.com/dotnet/roslyn/pull/26325

Right now the symbol model has IEnumerable<string> INamedTypeSymbo.MemberNames { get; }. However, both the VB and C# implementations implement that member with a simple string[] that is passed directly out to the caller. This means the caller can trivially cast this to the real type and mutate the array, corrupting the internal data of the compiler.

This returned value needs to be a true immutable value that cannot be changed by the caller in order to preserve hte important immutability invariants of the compiler API.

[CyrusNajmabadi]

Tagging @jcouv

[jcouv]

I'm tempted to say that if you tamper with the compilation, then yes, bad things will happen. You can probably use Reflection on returned symbols and do bad things too.
On the other hand, it seems an easy fix (add .AsImmutableArray()).
Tagging @gafter for thoughts.

[gafter]

Reflection is "outside the language" and can and should circumvent the language. However casts are not outside the language.

I agree: we should be returning an immutable array.

[CyrusNajmabadi]

I agree with neal. This is not a case of reflection. This is a case of casting a publicly exposed piece of data to a public mutable type, and having that succeed. Note: this is also fixed as part of: https://github.com/dotnet/roslyn/pull/26330

If https://github.com/dotnet/roslyn/pull/26325 goes in, we can then review that PR.