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.

Sign up for GitHub

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

Jump to bottom

isisd: fix crash when reading asla #16718

Merged
merged 1 commit into from
Sep 10, 2024
Merged

isisd: fix crash when reading asla #16718

merged 1 commit into from
Sep 10, 2024

Conversation

louis-6wind
Copy link
Contributor

isisd is crashing when reading a ASLA sub-TLV with Application Identifier Bit Mask length greater than 1 octet.

Set a limit of 8 bytes in accordance with RFC9479 and check that the received value does not exceed the limit.

Link: https://www.rfc-editor.org/rfc/rfc9479.html#name-application-identifier-bit-
Fixes: 5749ac8 ("isisd: add ASLA support")

ton31337
ton31337 reviewed Sep 2, 2024
View reviewed changes
isisd/isis_tlvs.c Outdated Show resolved Hide resolved
ton31337
ton31337 reviewed Sep 2, 2024
View reviewed changes
Copy link
Member

@ton31337 ton31337 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible to see the backtrace of the crash?

@louis-6wind louis-6wind force-pushed the fix-asla-length branch 2 times, most recently from b9f80d9 to 4a93aa2 Compare September 3, 2024 13:19
@louis-6wind
Copy link
Contributor Author

Possible to see the backtrace of the crash?

It was reported to me on slack. I have no backtrace

@ton31337
Copy link
Member

ton31337 commented Sep 3, 2024

@Mergifyio backport stable/10.1 stable/10.0 stable/9.1 stable/9.0

@mergify Mergify
Copy link

mergify bot commented Sep 3, 2024 •
edited
Loading

backport stable/10.1 stable/10.0 stable/9.1 stable/9.0

✅ Backports have been created

@donaldsharp
Copy link
Member

@louis-6wind I pointed out the backtrace to @ton31337 in slack. I don't believe you necessarily need a stack trace for this crash but if you do as well, I will send it to you in slack

@louis-6wind
Copy link
Contributor Author

louis-6wind commented Sep 3, 2024 •
edited
Loading

@louis-6wind I pointed out the backtrace to @ton31337 in slack. I don't believe you necessarily need a stack trace for this crash but if you do as well, I will send it to you in slack

No I am fine without the backtrace. Thanks

riw777
riw777 approved these changes Sep 3, 2024
View reviewed changes
Copy link
Member

@riw777 riw777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

@louis-6wind
isisd: fix crash when reading asla
f1bc6c5 
isisd is crashing when reading a ASLA sub-TLV with Application
Identifier Bit Mask length greater than 1 octet.

Set a limit of 8 bytes in accordance with RFC9479 and check that the
received value does not exceed the limit.

Reported-by: Iggy Frankovic <[email protected]>
Link: https://www.rfc-editor.org/rfc/rfc9479.html#name-application-identifier-bit-
Fixes: 5749ac8 ("isisd: add ASLA support")
Signed-off-by: Louis Scalbert <[email protected]>
@riw777
Copy link
Member

riw777 commented Sep 10, 2024

@ton31337 are we waiting on something here, or can I push this?

@ton31337
Copy link
Member

Waiting to be confirmed by the reporter.

@louis-6wind
Copy link
Contributor Author

@ton31337

Waiting to be confirmed by the reporter.

It was confimed on slack: https://frrouting.slack.com/archives/C07KB1HFG9G/p1725377157093039

Iggy [17 h 25]
Good morning everyone! @louis Scalbert thank you very much for the fix. I just tested it and the crash is now gone.

@ton31337
Copy link
Member

Then ✌️

odd22
odd22 approved these changes Sep 10, 2024
View reviewed changes
Copy link
Member

@odd22 odd22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@odd22 odd22 merged commit 571cca2 into FRRouting:master Sep 10, 2024
11 checks passed
This was referenced Sep 10, 2024
Merged
Merged
Merged
Merged
Jafaral added a commit that referenced this pull request Sep 11, 2024
@Jafaral
Merge pull request #16785 from FRRouting/mergify/bp/stable/9.1/pr-16718
c709528 
isisd: fix crash when reading asla (backport #16718)
Jafaral added a commit that referenced this pull request Sep 11, 2024
@Jafaral
Merge pull request #16783 from FRRouting/mergify/bp/stable/10.1/pr-16718
6704b38 
isisd: fix crash when reading asla (backport #16718)
ton31337 added a commit that referenced this pull request Sep 11, 2024
@ton31337
Merge pull request #16786 from FRRouting/mergify/bp/stable/9.0/pr-16718
b28f754 
isisd: fix crash when reading asla (backport #16718)
ton31337 added a commit that referenced this pull request Sep 11, 2024
@ton31337
Merge pull request #16784 from FRRouting/mergify/bp/stable/10.0/pr-16718
6a6acab 
isisd: fix crash when reading asla (backport #16718)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ton31337 ton31337 ton31337 left review comments

@odd22 odd22 odd22 approved these changes

@riw777 riw777 riw777 approved these changes

Assignees
No one assigned
Labels
backport bugfix isis master size/S
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@louis-6wind @ton31337 @donaldsharp @riw777 @odd22