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

Jump to bottom

Bug30041 029 #921

Closed
wants to merge 4 commits into from
Closed

Bug30041 029 #921

wants to merge 4 commits into from

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@nmathewson @coveralls @teor2345 @stoeckmann
@nmathewson
Copy link
Contributor

@nmathewson nmathewson commented Apr 9, 2019

No description provided.

stoeckmann and others added 3 commits Apr 9, 2019
@stoeckmann @nmathewson
Protect buffers against INT_MAX datalen overflows.
74b2bc4 
Many buffer functions have a hard limit of INT_MAX for datalen, but
this limitation is not enforced in all functions:

- buf_move_all may exceed that limit with too many chunks
- buf_move_to_buf exceeds that limit with invalid buf_flushlen argument
- buf_new_with_data may exceed that limit (unit tests only)

This patch adds some annotations in some buf_pos_t functions to
guarantee that no out of boundary access could occur even if another
function lacks safe guards against datalen overflows.

  [This is a backport of the submitted patch to 0.2.9, where the
  buf_move_to_buf and buf_new_with_data functions did not exist.]
@stoeckmann @nmathewson
Check return value of buf_move_to_buf for error.
0fa9530 
If the concatenation of connection buffer and the buffer of linked
connection exceeds INT_MAX bytes, then buf_move_to_buf returns -1 as an
error value.

This value is currently casted to size_t (variable n_read) and will
erroneously lead to an increasement of variable "max_to_read".

This in turn can be used to call connection_buf_read_from_socket to
store more data inside the buffer than expected and clogging the
connection buffer.

If the linked connection buffer was able to overflow INT_MAX, the call
of buf_move_to_buf would have previously internally triggered an integer
overflow, corrupting the state of the connection buffer.

Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
@nmathewson
Changes file for bug30041
c100115
@coveralls
Copy link

@coveralls coveralls commented Apr 9, 2019 •
edited

Pull Request Test Coverage Report for Build 4652

  • 2 of 6 (33.33%) changed or added relevant lines in 2 files are covered.
  • 22 unchanged lines in 4 files lost coverage.
  • Overall coverage decreased (-0.04%) to 50.121%
Changes Missing Coverage Covered Lines Changed/Added Lines %
src/or/buffers.c 2 3 66.67%
src/or/connection.c 0 3 0.0%
Files with Coverage Reduction New Missed Lines %
src/or/dirvote.c 1 68.55%
src/or/circuitstats.c 2 73.4%
src/or/shared_random.c 4 85.38%
src/or/routerparse.c 15 72.77%
Totals Coverage Status
Change from base Build 4616: -0.04%
Covered Lines: 29499
Relevant Lines: 58855
💛 - Coveralls

@nmathewson
fixup! Protect buffers against INT_MAX datalen overflows.
f100c55
@teor2345
Copy link
Contributor

@teor2345 teor2345 commented Aug 9, 2019

Obsoleted by #1229.

@teor2345 teor2345 closed this Aug 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment