erpc icon indicating copy to clipboard operation
erpc copied to clipboard

[BUG] wrong buffer length can retrieve

Open amgross opened this issue 2 years ago • 2 comments

Describe the bug

There are cases that length that got with readBinary (and readString also probably) will be overwritten by other readBinary or by reading the length.

It supposed to be overwritten with same length but attacker can send send whatever he want over the transport layer To Reproduce

Run erpcgen on example.erpc.txt

You will see:

  1. at my_example_two_buffers_shim
        codec->readBinary(&lengthTemp_0, &dataTemp_0);
        len = lengthTemp_0;
        ...
        codec->readBinary(&lengthTemp_1, &dataTemp_1);
        len = lengthTemp_1;
  1. at my_example_len_after_buffer_shim
        codec->readBinary(&lengthTemp_0, &dataTemp_0);
        len = lengthTemp_0;
        ...
        codec->read(&len);

Expected behavior

As mentioned in #327 , fixing for reading the length just once is break change and not acceptable. So I think the length should be compared instead of overwrite and in case it is not same to fail.

I don't know how to make this fix currently.

Screenshots

Desktop (please complete the following information):

  • OS: Linux
  • eRPC Version: 1.9.1

Steps you didn't forgot to do

  • [X] I checked if there is no related issue opened/closed. (#327 is related, but is different)
  • [X] I checked that there doesn't exist opened PR which is solving this issue.

Additional context

amgross avatar Dec 13 '22 14:12 amgross

Sounds good. But as mentioned i would like to get rid of one len size reading, which is breaking change, but i think we can do it and properly mark version.

Hadatko avatar Dec 13 '22 14:12 Hadatko

I think that for such little enhancement we shouldn't start breaking things. We can keep it in some backlog till there will be anyway breaking.

I suggest fixing current issue without breaking and to wait with #327

amgross avatar Dec 14 '22 05:12 amgross