Skip to content

HighPriorityASDUQueue_enqueue can overwrite queued ASDUs after wraparound #199

@gezhaoyes

Description

@gezhaoyes

Hi,

I think there is a wraparound bug in HighPriorityASDUQueue_enqueue in
lib60870-C/src/iec60870/cs104/cs104_slave.c.

Relevant code:
https://github.com/mz-automation/lib60870/blob/master/lib60870-C/src/iec60870/cs104/cs104_slave.c#L3455-L3577

    if (nextMsgPtr + entrySize > self->buffer + self->size)
    {
        nextMsgPtr = self->buffer;
        self->lastInBufferEntry = self->lastEntry;
    }

    if (self->entryCounter > 0)
    {
        if (nextMsgPtr <= self->firstEntry)
        {
            if (nextMsgPtr + entrySize > self->firstEntry)
            {
                enqueued = false;
            }
...

The function computes the next insertion position after lastEntry. If the new entry does not fit before the physical end of the buffer, it unconditionally wraps nextMsgPtr back to self->buffer and then checks the new entry only against self->firstEntry.

This is correct only when the queue is not already wrapped. If the queue is already wrapped, the low-address part of the buffer already contains queued entries.

Example state, using offsets relative to self->buffer:

buffer size = 1000
firstEntry = 900
lastInBufferEntry = 950
lastEntry = 800

At this point the queue is already wrapped. The occupied regions are approximately:

[900, 950...] and [0, lastEntry_end)

The only free region is between lastEntry_end and firstEntry.

Now assume the current last entry ends at offset 850 and the new entrySize is 200. The correct behavior should be to reject the enqueue, because it does not fit at the tail and the low-address region is already occupied.

But the current code wraps nextMsgPtr to offset 0. Since 0 + 200 <= firstEntry, enqueued remains true and the function writes the new ASDU at the beginning of the buffer, overwriting older queued high-priority ASDUs.

So the wraparound check needs to distinguish between:

queue not yet wrapped: tail overflow may wrap to the beginning if there is enough room before firstEntry
queue already wrapped (lastEntry < firstEntry): tail overflow means there is no valid insertion position, because the low-address region is occupied

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions