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

Fix off-by-one error in FindLocalNodeFromDestionationId #17942

Merged
merged 1 commit into from
May 2, 2022

Conversation

msandstedt
Copy link
Contributor

Problem

FindLocalNodeFromDestionationId is indexing 1 entry past the initialized IPK epoch keys, with the result that an all-zero key is accepted when one or two epoch-keys are installed. If three epoch keys are installed, this will reference out of bounds.

Change overview

This commit corrects the loop bound in this method to fix the problem.

Fixes #17940

Testing

Manually tested with an initiator using an incorrect, all-zero key. Without the fix, CASE establishment succeeds. With the fix, the responder now correctly rejects the incoming establishment request.

FindLocalNodeFromDestionationId is indexing 1 entry past the initialized
IPK epoch keys, with the result that an all-zero key is accepted when
one or two epoch-keys are installed.  If three epoch keys are installed,
this will reference out of bounds.

This commit corrects the loop bound in this method to fix the problem.

Testing: Manually tested with an initiator using an incorrect, all-zero
key.  Without the fix, CASE establishment succeeds.  With the fix, the
responder now correctly rejects the incoming establishment request.

Fixes project-chip#17940
@github-actions
Copy link

github-actions bot commented Apr 30, 2022

PR #17942: Size comparison from 5df9328 to 17d4a1f

Increases (1 build for esp32)
platform target config section 5df9328 17d4a1f change % change
esp32 all-clusters-app c3devkit (read only) 999604 999608 4 0.0
.flash.text 999604 999608 4 0.0
Decreases (1 build for telink)
platform target config section 5df9328 17d4a1f change % change
telink lighting-app tlsr9518adk80d text 571142 571140 -2 -0.0
Full report (25 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section 5df9328 17d4a1f change % change
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 688903 688903 0 0.0
(read/write) 163352 163352 0 0.0
.bss 75236 75236 0 0.0
.data 3400 3400 0 0.0
.rodata 102303 102303 0 0.0
.text 586116 586116 0 0.0
lock-ftd LP_CC2652R7 (read only) 676767 676767 0 0.0
(read/write) 166744 166744 0 0.0
.bss 73548 73548 0 0.0
.data 3224 3224 0 0.0
.rodata 94359 94359 0 0.0
.text 581928 581928 0 0.0
lock-mtd LP_CC2652R7 (read only) 625527 625527 0 0.0
(read/write) 146352 146352 0 0.0
.bss 69268 69268 0 0.0
.data 3224 3224 0 0.0
.rodata 94247 94247 0 0.0
.text 530792 530792 0 0.0
pump-app LP_CC2652R7 (read only) 661267 661267 0 0.0
(read/write) 183484 183484 0 0.0
.bss 73764 73764 0 0.0
.data 3256 3256 0 0.0
.rodata 80387 80387 0 0.0
.text 580396 580396 0 0.0
pump-controller-app LP_CC2652R7 (read only) 654155 654155 0 0.0
(read/write) 190396 190396 0 0.0
.bss 73820 73820 0 0.0
.data 3220 3220 0 0.0
.rodata 83323 83323 0 0.0
.text 570348 570348 0 0.0
cyw30739 light cyw930739m2evb_01 (read/write) 626354 626354 0 0.0
.app_xip_area 528888 528888 0 0.0
.bss 80116 80116 0 0.0
.data 696 696 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
lock cyw930739m2evb_01 (read/write) 625066 625066 0 0.0
.app_xip_area 529064 529064 0 0.0
.bss 78692 78692 0 0.0
.data 660 660 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 574194 574194 0 0.0
.app_xip_area 468572 468572 0 0.0
.bss 88016 88016 0 0.0
.data 572 572 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A (read only) 908216 908216 0 0.0
(read/write) 135128 135128 0 0.0
.bss 133072 133072 0 0.0
.data 2052 2052 0 0.0
.text 908208 908208 0 0.0
BRD4161A+rpc (read only) 942560 942560 0 0.0
(read/write) 151808 151808 0 0.0
.bss 149552 149552 0 0.0
.data 2256 2256 0 0.0
.text 942552 942552 0 0.0
BRD4161A+rs911x (read only) 746516 746516 0 0.0
(read/write) 129352 129352 0 0.0
.bss 127372 127372 0 0.0
.data 1980 1980 0 0.0
.text 746508 746508 0 0.0
lock-app BRD4161A+wf200 (read only) 916512 916512 0 0.0
(read/write) 127540 127540 0 0.0
.bss 125604 125604 0 0.0
.data 1936 1936 0 0.0
.text 916504 916504 0 0.0
window-app BRD4161A (read only) 845464 845464 0 0.0
(read/write) 133216 133216 0 0.0
.bss 131248 131248 0 0.0
.data 1964 1964 0 0.0
.text 845456 845456 0 0.0
esp32 all-clusters-app c3devkit (read only) 999604 999608 4 0.0
(read/write) 1474506 1474506 0 0.0
.dram0.bss 68376 68376 0 0.0
.dram0.data 14428 14428 0 0.0
.flash.rodata 207248 207248 0 0.0
.flash.text 999604 999608 4 0.0
.iram0.text 62020 62020 0 0.0
m5stack (read only) 1054795 1054795 0 0.0
(read/write) 476936 476936 0 0.0
.dram0.bss 73896 73896 0 0.0
.dram0.data 34176 34176 0 0.0
.flash.rodata 237028 237028 0 0.0
.flash.text 1049411 1049411 0 0.0
.iram0.text 123107 123107 0 0.0
k32w light k32w061+release (read/write) 684236 684236 0 0.0
.bss 81320 81320 0 0.0
.data 2008 2008 0 0.0
.text 599204 599204 0 0.0
lock k32w061+release (read/write) 729132 729132 0 0.0
.bss 81744 81744 0 0.0
.data 1968 1968 0 0.0
.text 643716 643716 0 0.0
linux chip-tool-no-interactive-ipv6only arm64 (read only) 8692244 8692244 0 0.0
(read/write) 616305 616305 0 0.0
.bss 40913 40913 0 0.0
.data 1192 1192 0 0.0
.data.rel.ro 555648 555648 0 0.0
.dynamic 560 560 0 0.0
.got 14736 14736 0 0.0
.init 24 24 0 0.0
.init_array 184 184 0 0.0
.rodata 429748 429748 0 0.0
.text 6865252 6865252 0 0.0
thermostat-no-ble arm64 (read only) 2360980 2360980 0 0.0
(read/write) 174593 174593 0 0.0
.bss 86273 86273 0 0.0
.data 1496 1496 0 0.0
.data.rel.ro 79048 79048 0 0.0
.dynamic 560 560 0 0.0
.got 4736 4736 0 0.0
.init 24 24 0 0.0
.init_array 376 376 0 0.0
.rodata 145748 145748 0 0.0
.text 1986080 1986080 0 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2418060 2418060 0 0.0
.bss 205884 205884 0 0.0
.data 5856 5856 0 0.0
.text 1380660 1380660 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1180107 1180107 0 0.0
bss 142000 142000 0 0.0
rodata 150804 150804 0 0.0
text 808620 808620 0 0.0
p6 all-clusters-app default (read/write) 2528576 2528576 0 0.0
.bss 139256 139256 0 0.0
.data 2792 2792 0 0.0
.text 1486840 1486840 0 0.0
light-app default (read/write) 2419176 2419176 0 0.0
.bss 132720 132720 0 0.0
.data 2592 2592 0 0.0
.text 1377440 1377440 0 0.0
lock-app default (read/write) 2428448 2428448 0 0.0
.bss 132544 132544 0 0.0
.data 2552 2552 0 0.0
.text 1386712 1386712 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 807348 807348 0 0.0
bss 75432 75432 0 0.0
noinit 40416 40416 0 0.0
text 571142 571140 -2 -0.0

Copy link
Contributor

@tcarmelveilleux tcarmelveilleux left a comment

Choose a reason for hiding this comment

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

Great catch

@bzbarsky-apple bzbarsky-apple merged commit cfb160f into project-chip:master May 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FindLocalNodeFromDestionationId uses uninitialized IPKs
4 participants