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

p2p: define DiscReason as uint8 #164

Merged
merged 2 commits into from
Oct 12, 2022
Merged

p2p: define DiscReason as uint8 #164

merged 2 commits into from
Oct 12, 2022

Conversation

iczc
Copy link

@iczc iczc commented Oct 8, 2022

cherry-pick form ethereum#24507 to fix CVE-2022-29177.

All other implementations store disconnect reasons as a single byte,
so go-ethereum should do it too.
@ChuhanJin
Copy link

Can one of the admins verify this patch?

@0xmountaintop
Copy link

close and re-open to trigger CI

@iczc
Copy link
Author

iczc commented Oct 8, 2022

Since DiscReason was an alias type of unit, the malicious node can build discMsg with math.MaxUint DiscReason and send it to a victim node.
A victim node if configured to use high verbosity logging(>3) will print the DiscReason, then the String() method of the DiscReason type will be called.

func (d DiscReason) String() string {
	if len(discReasonToString) <= int(d) {
		return fmt.Sprintf("unknown disconnect reason %d", d)
	}
	return discReasonToString[d]
}

func (d DiscReason) Error() string {
	return d.String()
}

As int(math.MaxUint) results in -1, the if branch will not be executed, and an "index out of range" runtime error PANIC will occur on return discReasonToString[d], this can cause the node to crash.

@iczc iczc merged commit b3a7c9b into staging Oct 12, 2022
@iczc iczc deleted the fix_p2p branch October 12, 2022 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants