isisd: fix rcap tlv double-free crash (backport #16809) #16845
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![mergify[bot]](https://avatars.githubusercontent.com/in/10562?s=60&v=4){kind=small}
A double-free crash happens when a subTLV of the "Router Capability" TLV is not readable and a previous "Router Capability" TLV was read. rcap was supposed to be freed later by isis_free_tlvs() -> free_tlv_router_cap(). In 78774bb ("isisd: add isis flex-algo lsp advertisement"), this was not the case because rcap was not saved to tlvs->router_cap when the function returned early because of a subTLV length issue. Always set tlvs->router_cap to free the memory. Note that this patch has the consequence that in case of subTLV error, the previously read "Router Capability" subTLVs are kept in memory. Fixes: 49efc80 ("isisd: Ensure rcap is freed in error case") Fixes: 78774bb ("isisd: add isis flex-algo lsp advertisement") Reported-by: Iggy Frankovic <[email protected]> Signed-off-by: Louis Scalbert <[email protected]> (cherry picked from commit d617581)
Since the previous commit, if a router capability subTLV is not readable, the previously read subTLVs are kept. Update of the ISIS fuzz test. > $ wuschl rebuild tests/isisd/test_fuzz_isis_tlv > $ gzip -9 tests/isisd/test_fuzz_isis_tlv_tests.h > $ ./test_fuzz_isis_tlv 2>/dev/null | grep failed > Test 139 failed, output differs. > Test 150 failed, output differs. > 2 of 405 tests failed. > > $ ./test_fuzz_isis_tlv 139 2>/dev/null > Test 139 failed, output differs. > Expected output: > Unpack log: > Unpacking 564 bytes of TLVs... > Unpacking TLV... > Found TLV of type 193 and len 13. > Skipping unknown TLV 193 (13 bytes) > Unpacking TLV... > Found TLV of type 0 and len 0. > Skipping unknown TLV 0 (0 bytes) > Unpacking TLV... > Found TLV of type 0 and len 0. > Skipping unknown TLV 0 (0 bytes) > Unpacking TLV... > Found TLV of type 242 and len 12. > Unpacking Router Capability TLV... > WARNING: Router Capability subTLV length too large compared to expected size > Unpacked TLVs: > Received output: > Unpack log: > Unpacking 564 bytes of TLVs... > Unpacking TLV... > Found TLV of type 193 and len 13. > Skipping unknown TLV 193 (13 bytes) > Unpacking TLV... > Found TLV of type 0 and len 0. > Skipping unknown TLV 0 (0 bytes) > Unpacking TLV... > Found TLV of type 0 and len 0. > Skipping unknown TLV 0 (0 bytes) > Unpacking TLV... > Found TLV of type 242 and len 12. > Unpacking Router Capability TLV... > WARNING: Router Capability subTLV length too large compared to expected size > Unpacked TLVs: > Router Capability: 253.212.128.242 , D:1, S:1 > > $ ./test_fuzz_isis_tlv 150 2>/dev/null > Test 150 failed, output differs. > Expected output: > Unpack log: > Unpacking 403 bytes of TLVs... > Unpacking TLV... > Found TLV of type 129 and len 13. > Unpacking Protocols Supported TLV... > Protocols Supported: 73, 16, 255, 255, 255, 101, 10, 11, 11, 11, 11, 11, 11 > Unpacking TLV... > Found TLV of type 11 and len 11. > Skipping unknown TLV 11 (11 bytes) > Unpacking TLV... > Found TLV of type 242 and len 12. > Unpacking Router Capability TLV... > WARNING: Router Capability subTLV length too large compared to expected size > Unpacked TLVs: > Protocols Supported: 73, 16, 255, 255, 255, 101, 10, 11, 11, 11, 11, 11, 11 > Received output: > Unpack log: > Unpacking 403 bytes of TLVs... > Unpacking TLV... > Found TLV of type 129 and len 13. > Unpacking Protocols Supported TLV... > Protocols Supported: 73, 16, 255, 255, 255, 101, 10, 11, 11, 11, 11, 11, 11 > Unpacking TLV... > Found TLV of type 11 and len 11. > Skipping unknown TLV 11 (11 bytes) > Unpacking TLV... > Found TLV of type 242 and len 12. > Unpacking Router Capability TLV... > WARNING: Router Capability subTLV length too large compared to expected size > Unpacked TLVs: > Protocols Supported: 73, 16, 255, 255, 255, 101, 10, 11, 11, 11, 11, 11, 11 > Router Capability: 253.212.128.242 , D:1, S:1 Link: https://pypi.org/project/wuschl/ Signed-off-by: Louis Scalbert <[email protected]> (cherry picked from commit cfd050a) # Conflicts: # tests/isisd/test_fuzz_isis_tlv_tests.h.gz
|
Cherry-pick of cfd050a has failed: To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix rcap tlv double-free crash. It is supposed to freed later by isis_free_tlvs() -> free_tlv_router_cap().
Fixes: 49efc80 ("isisd: Ensure rcap is freed in error case")
This is an automatic backport of pull request #16809 done by Mergify.