dotnet / dotnet/runtime

`ReadOnlyTensorSpan<T>` can read outside bounds of array

Open
#133,459 1 comment 1 reaction 2 assignees Claimed by @EgorBo View on GitHub
area-System.Numerics.Tensors untriaged
Dominant language
C#
Stars
18.3k
Forks
5.6k
PR merge metrics
PR metrics pending

Description

### Description

`ReadOnlyTensorSpan` is documented as type and memory safe, but the constructors that take System.Array (as opposed to `T[]`) are not actually safe.

### Reproduction Steps

```c#
#:package System.Numerics.Tensors@10.0.12
using System.Numerics.Tensors;

var bytes =new byte[1024*1024];

// ReadOnlyTensorSpan is documented as memory safe, but it's System.Array constructor
// isn't memory safe if the type parameter is not the same size as the array element.
var rots = new ReadOnlyTensorSpan((System.Array)bytes);

// AccessViolationException from reading well past end of bytes array
Console.WriteLine(rots[bytes.Length-1]);
```

### Expected behavior

The System.Array constructor for `ReadOnlyTensorSpan` should have thrown an `ArrayTypeMismatchException`, or something along those lines. I used the shorter constructor for the demo, but both are affected.

### Actual behavior

No element type validation occurred, which allowed reading outside the bounds of the array.

### Regression?

This is a regression. The bug was introduced in #114927, so landed in all the 10.0.x packages including many of the prelease versions.

### Known Workarounds

Use the strongly typed array constructors when you have a single dimensional zero indexed array (SZArray), and use the correct element type if dealing with an MDArray.

### Configuration

_No response_

### Other information

I suppose this could be a security issue if somebody has released code with such a type mismatch, but it also just wouldn't work correctly, so this would only happen if they shipped completely untested tensor code and happened to use this more obscure constructor that was intended for multi-dimensional arrays, which feels unlikely to me.

The fix cannot be perfectly backwards compatible, but either a strict element check like we had with 9.x or a less strict one that allows upcasting reference types would be fine. Silent reference type covariance was implicitly allowed for the `T[]` case, since that is safe for a read-only span.

The normal `TensorSpan` code appears correct, as it applies strict element type checks.

@tannergooding

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.