Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential bug: Presentation underflow #49

Open
rnd-ash opened this issue Jun 27, 2022 · 2 comments
Open

Potential bug: Presentation underflow #49

rnd-ash opened this issue Jun 27, 2022 · 2 comments

Comments

@rnd-ash
Copy link

rnd-ash commented Jun 27, 2022

Hey!

As you know, my own CBFParser shares a lot of similar code with CaesarSuite.

However, as CBFParser is built in Rust and not C#, I thought I would alert you to a potential bug (Not tested in Caesarsuite), which trips Rusts debug build underflow detection. The code in question is shared between CBFParser and CaesarSuite.

Parsing CPC301T.CBF panics like so:

Error:
 × Main thread panicked.
 ├─▶ at C:\Users\___\.cargo\git\checkouts\openvehiclediag-
 │   39994c2d0095f930\09719e6\CBFParser\src\diag\preparation.rs:139:35
 ╰─▶ attempt to subtract with overflow
 help: set the `RUST_BACKTRACE=1` environment variable to display a backtrace.

This panic is caused by this line:
https://github.com/rnd-ash/OpenVehicleDiag/blob/egui_ecu_diag/CBFParser/src/diag/preparation.rs#L139

result_bit_size =(((byte_count & 0xFF) - (self.bit_pos / 8)) * 8) as i32;

So essentially, byte_count & 0xFF is somehow smaller than bit_pos, which triggers the underflow detection, breaking the parser.

I noticed that CaesarSuite shares almost identical code here:
https://github.com/jglim/CaesarSuite/blob/main/Caesar/Caesar/DiagPreparation.cs#L246

int resultByteSize = (ParentDiagService.RequestBytes.Length & 0xFF) - (BitPosition / 8);

So I've raised this issue here in case this is something that CaesarSuite does not handle correctly and instead continues silently (I don't program in C# so not sure if it has a similar Over/underflow detection system :) )

@jglim
Copy link
Owner

jglim commented Jun 29, 2022

The library is built without /unsafe so it automatically includes runtime bounds checking that is provided by .net. In the case of a malformed file that leads to this issue, the runtime will make angry noises and exit without memory corruption or losing control flow.

For now, I can't replicate this as I do not have a copy of CPC301T.CBF. I will not be making any changes, but will keep an eye out for this particular case.

Thanks for looking out for this project ^^

@N0cynym
Copy link
Contributor

N0cynym commented Jun 29, 2022

If you have a Xentry installation you'll find this file at this path Program Files\Mercedes-Benz\DAS\comdat\evobus\cbf

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants