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

[NCC-E005955-HV6] zebra-network: Buffer length validation after memory allocation #6280

Closed
Tracked by #6277
mpguerra opened this issue Mar 9, 2023 · 1 comment · Fixed by #6320
Closed
Tracked by #6277
Assignees
Labels
C-audit Category: Issues arising from audit findings C-security Category: Security issues

Comments

@mpguerra
Copy link
Contributor

mpguerra commented Mar 9, 2023

Motivation

We want to track all of the findings from the zebra audit.

Details

When deserializing a version 2 address, the length of the (variable-length) address is validated after the address is read into a buffer. An unexpectedly large address can temporarily lead to a large memory allocation on the heap. It is recommended to peek at the length of the address and validate it before reading the addr:

// > CompactSize The length in bytes of addr.
// > uint8[sizeAddr] Network address. The interpretation depends on networkID.
let addr: Vec<u8> = (&mut reader).zcash_deserialize_into()?;
// > uint16 Network port. If not relevant for the network this MUST be 0.
let port = reader.read_u16::<BigEndian>()?;
if addr.len() > MAX_ADDR_V2_ADDR_SIZE {
return Err(SerializationError::Parse(
"addr field longer than MAX_ADDR_V2_ADDR_SIZE in addrv2 message",
));
}

Note that the deserialization logic ensures that the length of the addr vector is smaller than MAX_U8_ALLOCATION (currently 2097147 bytes) which is significantly higher than MAX_ADDR_V2_ADDR_SIZE (currently set to 512).

@mpguerra mpguerra added this to Zebra Mar 9, 2023
@github-project-automation github-project-automation bot moved this to 🆕 New in Zebra Mar 9, 2023
@mpguerra mpguerra added C-audit Category: Issues arising from audit findings S-needs-triage Status: A bug report needs triage P-Medium ⚡ labels Mar 9, 2023
@mpguerra
Copy link
Contributor Author

@oxarbitrage oxarbitrage added the C-security Category: Security issues label Mar 14, 2023
@mergify mergify bot closed this as completed in #6320 Mar 15, 2023
@github-project-automation github-project-automation bot moved this from 🆕 New to ✅ Done in Zebra Mar 15, 2023
@mpguerra mpguerra changed the title zebra-network: Buffer length validation after memory allocation [NCC-E005955-HV6] zebra-network: Buffer length validation after memory allocation Mar 16, 2023
@mpguerra mpguerra removed the S-needs-triage Status: A bug report needs triage label Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-audit Category: Issues arising from audit findings C-security Category: Security issues
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants