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

Incorporate OOM fix in next release (v2.6.0) #411

Closed
smoelius opened this issue May 14, 2019 · 8 comments
Closed

Incorporate OOM fix in next release (v2.6.0) #411

smoelius opened this issue May 14, 2019 · 8 comments

Comments

@smoelius
Copy link

Please incorporate commit 66a22c8 from master in the next release (v2.6.0).

In particular, please incorporate the following OOM fix:
66a22c8#diff-03da03412d4490720c45da0a6f43d56cR36

// Max allocated vec when reading length-delimited from unknown input stream
const READ_RAW_BYTES_MAX_ALLOC: usize = 10_000_000;

66a22c8#diff-03da03412d4490720c45da0a6f43d56cR640
if count >= READ_RAW_BYTES_MAX_ALLOC {
// avoid calling `reserve` on buf with very large buffer: could be a malformed message
let mut take = self.by_ref().take(count as u64);
take.read_to_end(target)?;
if target.len() != count {
return Err(ProtobufError::WireError(WireError::TruncatedMessage));
}
} else {
target.reserve(count);
unsafe {
target.set_len(count);
}
self.source.read_exact(target)?;

We hit this OOM error while fuzzing a project that uses the current release, v2.5.0. Patching v2.5.0 with the above commit made the error go away. It would be convenient if the commit were incorporated in an official release.

Thank you.

@stepancheg
Copy link
Owner

Cherry-picked into v2.6 branch as f32ac1c.

@stepancheg
Copy link
Owner

Published version 2.6.0.

@dvdplm
Copy link

dvdplm commented May 19, 2019

Would you consider back porting this fix to 1.x?

@stepancheg
Copy link
Owner

Merged into v1.7 branch as a839f7b. Testing...

@stepancheg
Copy link
Owner

Published 1.7.5 with the same fix.

stepancheg added a commit that referenced this issue May 20, 2019
Nobody noticed likely because `with-bytes` feature is rarely used.

CC: #411
@stepancheg
Copy link
Owner

stepancheg commented May 20, 2019

FYI ef888e0 fixes the same issue when using both feature with-bytes and carllerche_bytes_for_xxx codegen option (so it affects only few people).

Need to backport it 2.6 and 1.7 too.

@stepancheg stepancheg reopened this May 20, 2019
@smoelius
Copy link
Author

Thanks, @stepancheg !

stepancheg added a commit that referenced this issue Jun 3, 2019
Nobody noticed likely because `with-bytes` feature is rarely used.

CC: #411
stepancheg added a commit that referenced this issue Jun 3, 2019
Nobody noticed likely because `with-bytes` feature is rarely used.

CC: #411
@stepancheg
Copy link
Owner

Published 2.6.2 with a fix for bytes::Bytes fields.

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

No branches or pull requests

3 participants