Possible Security Vulnerability of broadcast_to_ops.cc in tensorflow v2.7.0- kernel module

[cse cse]

Hi all,

While I was testing tensorflow kernel tests, I found that in broadcast_to_ops.cc file,  the checker at lines 81-84, is not covered by unit tests implemented in broadcast_to_ops_test.py. It is possible that an attacker craft an special input to BroadcastToOp since the checker is not working as it is expected to do. I think the the checker itself is a security hole.

If you want to reproduce the vulnerability, first you need to build TensorFlow from source (please checkout v2.7.0) using bazel, then remove lines 81-84, and finally run broadcast_to_ops_test.py.

Thanks,

Nima.

[cse cse]

Steps to reproduce the vulnerability:

# you need to have bazel installed

# Step 1: please clone tensorflow from github to your directory

# step 2: cd to your directory and run git checkout v2.7.0

# step 3: run configure.py and set configurations to default modes

# step 4: run the following commands in non virual environments

pip3 install --user -U pip six 'numpy<1.19.0' wheel setuptools mock 'future>=0.17.1'
pip3 install --user -U keras_applications --no-deps

pip3 install --user -U keras_preprocessing --no-deps

# Remove lines 81-84 from broadcast_to_ops.cc.

# run the following command in the current tensorflow directory you have cloned.

bazel build --config=opt -c opt //tensorflow/tools/pip_package:build_pip_package --jobs=4

# built a pip version of tensorflow

./bazel-bin/tensorflow/tools/pip_package/build_pip_package /path/to/your/desired/directory

# cd to /path/to/your/desired/directory and run the following commands

pip uninstall --yes tensorflow

pip install tensorflow-2.7.0-cp38-cp38-linux_x86_64.whl

[Mihai Maruseac]

Hi. Please share security issues with TF at the specific list, secu...@tensorflow.org.

Will look into the report, but also please take a look at https://github.com/tensorflow/tensorflow/blob/master/SECURITY.md to see what we consider a vulnerability and other general information about security in TF.

Thank you

--
You received this message because you are subscribed to the Google Groups "Discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to discuss+u...@tensorflow.org.
To view this discussion on the web visit https://groups.google.com/a/tensorflow.org/d/msgid/discuss/101b3e4f-3709-4136-aa56-a0f465bd3107n%40tensorflow.org.

--

Tensors must flow securely

[Mihai Maruseac]

Hi.

This is not a vulnerability, just a scenario that is not covered by any test.

Thank you

[cse cse]

Hi,

Thanks for the response.

I thought an attacker can craft special inputs (incompatible shapes) to BroadcastToOp, and obviously the checker in lines 81-84 is not covered by test cases and finally leading to out of bound read or seg fault.

Thanks,

Nima.

[Mihai Maruseac]

The invalid inputs would be caught by the existing lines.

[Mihai Maruseac]

Hi.

To give a more detailed explanation, this is similar to having a code such as

StatusOr<int> fun(int *array, int size, int ix) {

  // check here

  if (ix < 0 || ix >= size)

    return error::InvalidArgument("invalid index");

  return array[ix];

}

but only tests that execute this API with an index in bounds. Removing the check would still make all tests pass but will actually make the code worse as now someone would be able to call this with invalid index and read from outside the bounds. If the check is there, the malicious API call will properly return the error status.

So, to summarize: having a check that is not covered by a test is not a vulnerability. Removing the check only because there is no test that covers it *might* create a code weakness (might, because there are cases when the check is actually a duplicate of another one, but that is not the case in the original report).

Thank you

[cse cse]

Thanks for clarification. It was so clear and helpful for me.

I will report any uncovered code regions by your test suits discovered by my tool. I hope it helps you to increase the reliability of amazing Tensorflow.

Thanks,

Nima.