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

Issue with Data Length unmarshalling #92

Open
Javiplz opened this issue Jan 29, 2024 · 5 comments
Open

Issue with Data Length unmarshalling #92

Javiplz opened this issue Jan 29, 2024 · 5 comments

Comments

@Javiplz
Copy link

Javiplz commented Jan 29, 2024

There is an issue related with the unmarshal function in incomingMessage if the dis packet is malformed.
Application crashes with some udp packets with malformed lenght field.

In concrete it seems that occurs if the lenght is greater than expected at this location:

void Pdu::unmarshal(DataStream& dataStream)
{
dataStream >> _protocolVersion;
dataStream >> _exerciseID;
dataStream >> _pduType;
dataStream >> _protocolFamily;
dataStream >> _timestamp;
dataStream >> _length; // <----- length malformed , greater than expected
dataStream >> _padding;
}

After this code execution, the exectution continues in the final class (i.e. EntityStatePdu::unmarshall) and the application crashes. I think It is because the malformed length, as it seems to be used en EntityStatePdu...

This is an example of a udp packet causing a crash: 0x020001020000001d2500167374642d7363616e....

@chuismiguel
Copy link

Same issue here.

@leif81 leif81 added the bug label Feb 1, 2024
@Duxy1996
Copy link

Duxy1996 commented Feb 7, 2024

I'm having the same issue. The length of the PDU is not the real one.

@leif81
Copy link
Member

leif81 commented Feb 23, 2024

Thank-you for the bug report and comments.

I've been swamped with other projects. Would anyone be interested in submitting a pull request that resolve this? I'd be happy to review and merge it.

@Javiplz
Copy link
Author

Javiplz commented Mar 8, 2024

I could add some basic protection against this, just an extra check.

@Javiplz
Copy link
Author

Javiplz commented Mar 11, 2024

This is my proposal (src/dis6/Pdu.cpp, line 111):

void Pdu::unmarshal(DataStream& dataStream)
{
    dataStream >> _protocolVersion;
    dataStream >> _exerciseID;
    dataStream >> _pduType;
    dataStream >> _protocolFamily;
    dataStream >> _timestamp;
    dataStream >> _length;
    dataStream >> _padding;
    if (dataStream.size() != _length)    // added lines
    {
        throw std::runtime_error("error size"); 
    }
}

Regards.

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

No branches or pull requests

4 participants