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

babeld: bugs in parse_hello_subtlv, parse_ihu_subtlv, and parse_update_subtlv #10503

Closed
qingkaishi opened this issue Feb 4, 2022 · 6 comments · Fixed by #10504
Closed

babeld: bugs in parse_hello_subtlv, parse_ihu_subtlv, and parse_update_subtlv #10503

qingkaishi opened this issue Feb 4, 2022 · 6 comments · Fixed by #10504
Labels
triage Needs further investigation

Comments

@qingkaishi
Copy link
Contributor

frr/babeld/message.c

Lines 131 to 151 in e743c1b

parse_update_subtlv(const unsigned char *a, int alen,
unsigned char *channels)
{
int type, len, i = 0;
while(i < alen) {
type = a[i];
if(type == SUBTLV_PAD1) {
i++;
continue;
}
if(i + 1 > alen) {
flog_err(EC_BABEL_PACKET, "Received truncated attributes.");
return;
}
len = a[i + 1];
if(i + len > alen) {
flog_err(EC_BABEL_PACKET, "Received truncated attributes.");
return;
}

Line 143: the condition should be i + 1 >= alen instead of i + 1 > alen. Otherwise, overflows will happen at 147.

Line 148: the condition should be i + len + 2 > alen instead of i + len > alen. We need include extra two bytes, a[i] and a[i + 1] in this check.

@qingkaishi qingkaishi added the triage Needs further investigation label Feb 4, 2022
@qingkaishi
Copy link
Contributor Author

frr/babeld/message.c

Lines 179 to 201 in e743c1b

parse_hello_subtlv(const unsigned char *a, int alen,
unsigned int *hello_send_us)
{
int type, len, i = 0, ret = 0;
while(i < alen) {
type = a[0];
if(type == SUBTLV_PAD1) {
i++;
continue;
}
if(i + 1 > alen) {
flog_err(EC_BABEL_PACKET,
"Received truncated sub-TLV on Hello message.");
return -1;
}
len = a[i + 1];
if(i + len > alen) {
flog_err(EC_BABEL_PACKET,
"Received truncated sub-TLV on Hello message.");
return -1;
}

Line 185: it should be a[i] instead of a[0].

Line 191: the condition should be i + 1 >= alen instead of i + 1 > alen

Line 197: the condition should be i + len + 2 > alen instead of i + len > alen

@qingkaishi
Copy link
Contributor Author

qingkaishi commented Feb 4, 2022

frr/babeld/message.c

Lines 224 to 247 in e743c1b

parse_ihu_subtlv(const unsigned char *a, int alen,
unsigned int *hello_send_us,
unsigned int *hello_rtt_receive_time)
{
int type, len, i = 0, ret = 0;
while(i < alen) {
type = a[0];
if(type == SUBTLV_PAD1) {
i++;
continue;
}
if(i + 1 > alen) {
flog_err(EC_BABEL_PACKET,
"Received truncated sub-TLV on IHU message.");
return -1;
}
len = a[i + 1];
if(i + len > alen) {
flog_err(EC_BABEL_PACKET,
"Received truncated sub-TLV on IHU message.");
return -1;
}

Line 231: it should be a[i] instead of a[0].

Line 237: the condition should be i + 1 >= alen instead of i + 1 > alen.

Line 243: the condition should be i + len + 2 > alen instead of i + len > alen.

@idryzhov
Copy link
Contributor

idryzhov commented Feb 4, 2022

Hi @qingkaishi, thanks for letting everyone know about the issues.
But as you already know how to fix all of them, it would be much easier for us if you provide a PR with the fixes instead.

@qingkaishi
Copy link
Contributor Author

Hi @qingkaishi, thanks for letting everyone know about the issues. But as you already know how to fix all of them, it would be much easier for us if you provide a PR with the fixes instead.

Sure. I will do that soon.

@idryzhov
Copy link
Contributor

idryzhov commented Feb 4, 2022

Thanks!

qingkaishi added a commit to qingkaishi/frr that referenced this issue Feb 4, 2022
…n length

This patch repairs the checking conditions on length in four functions:
babel_packet_examin, parse_hello_subtlv, parse_ihu_subtlv, and parse_update_subtlv

Signed-off-by: qingkaishi <qingkaishi@gmail.com>
qingkaishi added a commit to qingkaishi/frr that referenced this issue Feb 4, 2022
…n length

This patch repairs the checking conditions on length in four functions:
babel_packet_examin, parse_hello_subtlv, parse_ihu_subtlv, and parse_update_subtlv

Signed-off-by: qingkaishi <qingkaishi@gmail.com>
mergify bot pushed a commit that referenced this issue Feb 8, 2022
This patch repairs the checking conditions on length in four functions:
babel_packet_examin, parse_hello_subtlv, parse_ihu_subtlv, and parse_update_subtlv

Signed-off-by: qingkaishi <qingkaishi@gmail.com>
(cherry picked from commit c379335)
plsaranya pushed a commit to plsaranya/frr that referenced this issue Feb 28, 2022
…n length

This patch repairs the checking conditions on length in four functions:
babel_packet_examin, parse_hello_subtlv, parse_ihu_subtlv, and parse_update_subtlv

Signed-off-by: qingkaishi <qingkaishi@gmail.com>
@qlyoung
Copy link
Member

qlyoung commented Mar 28, 2022

This has been assigned CVE-2022-26129 with a severity score of 7.8.

No assessment of exploitability has been made.

Please see my comment here.

patrasar pushed a commit to patrasar/frr that referenced this issue Apr 28, 2022
…n length

This patch repairs the checking conditions on length in four functions:
babel_packet_examin, parse_hello_subtlv, parse_ihu_subtlv, and parse_update_subtlv

Signed-off-by: qingkaishi <qingkaishi@gmail.com>
gpnaveen pushed a commit to gpnaveen/frr that referenced this issue Jun 7, 2022
…n length

This patch repairs the checking conditions on length in four functions:
babel_packet_examin, parse_hello_subtlv, parse_ihu_subtlv, and parse_update_subtlv

Signed-off-by: qingkaishi <qingkaishi@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage Needs further investigation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants