std.socket.Socket.receive is marked @trusted; however, this is unjustified, because it receives a void[] as buffer and overwrites the buffer with incoming socket data. If the buffer is an array of elements with indirection, this will break @safe-ty:
----
void main() @safe {
Socket sock = ...;
Object[] buf = new Object[1];
// This overwrites the Object ptr with arbitrary data:
sock.receive(buf, SocketFlags.init);
}
----
This raises the question of whether *any* function that takes in a non-const void[] can reasonably be marked @trusted, since the original type of the array has been erased and the function has no way to guarantee that writing to the void[] won't overwrite pointers with illegal values.
Comment #1 by hsteoh — 2016-02-18T16:20:18Z
This problem is made much worse by https://issues.dlang.org/show_bug.cgi?id=15672, because that allows the following truly evil code:
------
void readData(void[] buffer) @safe
{
ubyte[] buf = cast(ubyte[]) buffer; // why does this compile?!
buf[0] = 0xFF;
}
void main() @safe
{
auto buffer = new Object[1];
readData(buffer);
}
------
Thus, the @safe annotations here guarantee nothing at all.
Comment #2 by issues.dlang — 2016-02-18T18:35:05Z
I would think that converting from T[] to void[] would be @safe. That conversion won't actually corrupt anything. It's doing anything with the void[] which is the problem. Anything and everything which would involve interpreting what void[] is should definitely be @system.
Comment #3 by hsteoh — 2016-02-18T18:38:36Z
Yes, I agree. Converting T[] to void[] is @safe, but doing basically anything with the void[] other than reading it must be @system. Which means std.socket.Socket.receive should be @system, not @trusted. At least, it cannot be @trusted unless it verifies via sig constraints that hasIndirections!T is false.
Unfortunately changing this will probably break existing code, and it ain't gonna be pretty.
Comment #4 by hsteoh — 2016-02-18T18:51:20Z
Here's another example that shows why @safe/@trusted on any function that takes (a non-const) void[] must be considered suspect (credit: Steven Schveighoffer, from the forum thread):
----
void foo(void[] arr) @safe
{
void[] arr2 = [123, 456, 789]; // this is clearly @safe
arr[] = arr2[0 .. arr.length]; // so is this, under the current definition
}
----
Changing std.socket.Socket.receive to use templates to check for array indirections will break too much code, and does not play nice with inheritance.
Proposed alternative solution is to make it illegal to implicitly convert T[] to void[] in @safe code if T has indirections. As a compromise, continue to allow explicit cast to void[]. This will plug this particular hole as well as highlight potentially dangerous implicit conversions to void[], but still continue to allow it if the user explicitly casts to void[]. Seems like a reasonable compromise.