Skip to content

Commit

Permalink
Validate PSK index in pattern to avoid panic
Browse files Browse the repository at this point in the history
Previously, when the handshake tokens were compiled from the parsed
Noise parameters, snow would panic by attempting to access an
out-of-bounds index.
  • Loading branch information
mcginty committed Jan 26, 2024
1 parent 74e30cf commit 8b1a819
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 20 deletions.
10 changes: 5 additions & 5 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::fmt;

/// All errors in snow will include an `ErrorKind`.
#[allow(missing_docs)]
#[derive(Debug)]
#[derive(Debug, PartialEq)]
#[non_exhaustive]
pub enum Error {
/// The noise pattern failed to parse.
Expand Down Expand Up @@ -36,7 +36,7 @@ pub enum Error {
/// The various stages of initialization used to help identify
/// the specific cause of an `Init` error.
#[allow(missing_docs)]
#[derive(Debug)]
#[derive(Debug, PartialEq)]
pub enum PatternProblem {
TooFewParameters,
UnsupportedHandshakeType,
Expand All @@ -59,7 +59,7 @@ impl From<PatternProblem> for Error {
/// The various stages of initialization used to help identify
/// the specific cause of an `Init` error.
#[allow(missing_docs)]
#[derive(Debug)]
#[derive(Debug, PartialEq)]
pub enum InitStage {
ValidateKeyLengths,
ValidatePskLengths,
Expand All @@ -81,7 +81,7 @@ impl From<InitStage> for Error {

/// A prerequisite that may be missing.
#[allow(missing_docs)]
#[derive(Debug)]
#[derive(Debug, PartialEq)]
pub enum Prerequisite {
LocalPrivateKey,
RemotePublicKey,
Expand All @@ -95,7 +95,7 @@ impl From<Prerequisite> for Error {

/// Specific errors in the state machine.
#[allow(missing_docs)]
#[derive(Debug)]
#[derive(Debug, PartialEq)]
pub enum StateProblem {
MissingKeyMaterial,
MissingPsk,
Expand Down
10 changes: 10 additions & 0 deletions src/params/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,4 +309,14 @@ mod tests {
_ => panic!("missing token!"),
}
}

#[test]
fn test_invalid_psk_handshake() {
let p: NoiseParams = "Noise_XXpsk9_25519_AESGCM_SHA256".parse().unwrap();

assert_eq!(
Error::Pattern(PatternProblem::InvalidPsk),
HandshakeTokens::try_from(&p.handshake).unwrap_err()
);
}
}
28 changes: 13 additions & 15 deletions src/params/patterns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,7 @@ impl<'a> TryFrom<&'a HandshakeChoice> for HandshakeTokens {
#[allow(clippy::cognitive_complexity)]
fn try_from(handshake: &'a HandshakeChoice) -> Result<Self, Self::Error> {
// Hfs cannot be combined with one-way handshake patterns
#[cfg(feature = "hfs")]
check_hfs_and_oneway_conflict(handshake)?;

#[rustfmt::skip]
Expand Down Expand Up @@ -489,7 +490,7 @@ impl<'a> TryFrom<&'a HandshakeChoice> for HandshakeTokens {

for modifier in handshake.modifiers.list.iter() {
match modifier {
HandshakeModifier::Psk(n) => apply_psk_modifier(&mut patterns, *n),
HandshakeModifier::Psk(n) => apply_psk_modifier(&mut patterns, *n)?,
#[cfg(feature = "hfs")]
HandshakeModifier::Hfs => apply_hfs_modifier(&mut patterns),
_ => return Err(PatternProblem::UnsupportedModifier.into()),
Expand Down Expand Up @@ -517,21 +518,18 @@ fn check_hfs_and_oneway_conflict(handshake: &HandshakeChoice) -> Result<(), Erro
}
}

#[cfg(not(feature = "hfs"))]
fn check_hfs_and_oneway_conflict(_: &HandshakeChoice) -> Result<(), Error> {
Ok(())
}

fn apply_psk_modifier(patterns: &mut Patterns, n: u8) {
match n {
0 => {
patterns.2[0].insert(0, Token::Psk(n));
},
_ => {
let i = (n as usize) - 1;
patterns.2[i].push(Token::Psk(n));
},
/// Given our PSK modifier, we inject the token at the appropriate place.
fn apply_psk_modifier(patterns: &mut Patterns, n: u8) -> Result<(), Error> {
let tokens = patterns
.2
.get_mut((n as usize).saturating_sub(1))
.ok_or(Error::Pattern(PatternProblem::InvalidPsk))?;
if n == 0 {
tokens.insert(0, Token::Psk(n));
} else {
tokens.push(Token::Psk(n));
}
Ok(())
}

#[cfg(feature = "hfs")]
Expand Down

0 comments on commit 8b1a819

Please sign in to comment.