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

ber.go: Protocol Validation and Defense #81

Closed

Conversation

jentfoo
Copy link

@jentfoo jentfoo commented Jul 21, 2023

This commit attempts to address possible panic conditions due to access outside of the bounds of the slice. There are specifically three changes:

  • A couple log messages are commented out due to them attempting to do an out of bounds view into the slice.
  • Existing protocol validation was fixed. There was an attempt to guard against these conditions, however in many cases there was an off by one error, or other conditions had not been considered.
  • Because of the relevance around protocol validation, PR fix BER tag length check when length is between 0x80 and 0xFF #68 was also included as part of this work.

This commit attempts to address possible `panic` conditions due to access outside of the bounds of the slice.  There are specifically three changes:
 * A couple log messages are commented out due to them attempting to do an out of bounds view into the slice.
 * Existing protocol validation was fixed.  There was an attempt to guard against these conditions, however in many cases there was an off by one error, or other conditions had not been considered.
 * A PR from the parent fork which attempts to bring in a protocol fix for lengths between `0x80` and `0xFF` was also included as part of this: mozilla-services#68

The logic for slice access has been reviewed carefully in an attempt to make sure we are defensive, but still allowing the nuance of the PKCS7 structure.  In addition extensive fuzzing has been conducted on these changes.
@jentfoo jentfoo force-pushed the jent/protocol_defense branch from 7af100e to 69727ad Compare August 17, 2023 16:04
@ctbmozilla-admin ctbmozilla-admin added the ARCHIVED CLOSED at time of archiving label Feb 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ARCHIVED CLOSED at time of archiving
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants