Added checks to PNG validator to avoid overflows and to return early …

…when an invalid chunk type is detected
Will-Banksy · Feb 9, 2024 · adcdeca · adcdeca
1 parent 0965ce2
commit adcdeca
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 10 deletions.
diff --git a/libsearchlight/src/validation.rs b/libsearchlight/src/validation.rs
@@ -22,7 +22,7 @@ pub enum FileValidationType {
 	Correct,
 	Partial,
 	FormatError,
-	Corrupted,
+	Corrupt,
 	Unrecognised,
 	Unanalysed
 }
@@ -35,7 +35,7 @@ impl FileValidationType {
 			other
 		} else if self == FileValidationType::FormatError && other != FileValidationType::Correct && other != FileValidationType::Partial {
 			other
-		} else if self == FileValidationType::Corrupted && other != FileValidationType::Correct && other != FileValidationType::Partial && other != FileValidationType::FormatError {
+		} else if self == FileValidationType::Corrupt && other != FileValidationType::Correct && other != FileValidationType::Partial && other != FileValidationType::FormatError {
 			other
 		} else {
 			self
@@ -49,7 +49,7 @@ impl Display for FileValidationType {
 			FileValidationType::Correct=>"correct",
 			FileValidationType::Partial => "partial",
 			FileValidationType::FormatError => "format_error",
-			FileValidationType::Corrupted => "corrupted",
+			FileValidationType::Corrupt => "corrupted",
 			FileValidationType::Unrecognised => "unrecognised",
 			FileValidationType::Unanalysed => "unanalysed",
 		})

diff --git a/libsearchlight/src/validation/jpeg.rs b/libsearchlight/src/validation/jpeg.rs
@@ -65,7 +65,7 @@ impl FileValidator for JpegValidator {
 					}
 
 					break FileValidationInfo {
-						validation_type: FileValidationType::Corrupted,
+						validation_type: FileValidationType::Corrupt,
 						file_len: None
 					}
 				} else {

diff --git a/libsearchlight/src/validation/png.rs b/libsearchlight/src/validation/png.rs
@@ -14,7 +14,7 @@ pub struct PngValidator;
 struct ChunkValidationInfo {
 	validation_type: FileValidationType,
 	data_length: u32,
-	chunk_type: u32
+	chunk_type: u32,
 }
 
 impl PngValidator {
@@ -26,6 +26,17 @@ impl PngValidator {
 		let chunk_data_len = u32::from_be_bytes(data[0..4].try_into().unwrap());
 		let chunk_type = u32::from_be_bytes(data[4..8].try_into().unwrap());
 
+		// In the PNG spec, a valid chunk type must have each byte match [a-zA-Z]
+		let chunk_type_valid = chunk_type.to_ne_bytes().iter().all(|&b| (b'a' <= b && b <= b'z') || (b'A' <= b && b <= b'Z'));
+
+		if !chunk_type_valid || chunk_data_len as usize + 12 >= data.len() {
+			return ChunkValidationInfo {
+				validation_type: FileValidationType::Unrecognised,
+				data_length: 0,
+				chunk_type
+			};
+		}
+
 		let crc = u32::from_be_bytes(data[(chunk_data_len as usize + 8)..(chunk_data_len as usize + 12)].try_into().unwrap());
 
 		let calc_crc = crc32fast::hash(&data[4..(8 + chunk_data_len as usize)]);
@@ -64,7 +75,7 @@ impl PngValidator {
 				};
 
 				ChunkValidationInfo {
-					validation_type: if spec_conformant && chunk_intact { FileValidationType::Correct } else if chunk_intact { FileValidationType::FormatError } else { FileValidationType::Corrupted },
+					validation_type: if spec_conformant && chunk_intact { FileValidationType::Correct } else if chunk_intact { FileValidationType::FormatError } else { FileValidationType::Corrupt },
 					data_length: chunk_data_len,
 					chunk_type
 				}
@@ -73,14 +84,14 @@ impl PngValidator {
 				let spec_conformant = chunk_data_len % 3 == 0;
 
 				ChunkValidationInfo {
-					validation_type: if spec_conformant && chunk_intact { FileValidationType::Correct } else if chunk_intact { FileValidationType::FormatError } else { FileValidationType::Corrupted },
+					validation_type: if spec_conformant && chunk_intact { FileValidationType::Correct } else if chunk_intact { FileValidationType::FormatError } else { FileValidationType::Corrupt },
 					data_length: chunk_data_len,
 					chunk_type
 				}
 			}
 			_ => {
 				ChunkValidationInfo {
-					validation_type: if chunk_intact { FileValidationType::Correct } else { FileValidationType::Corrupted },
+					validation_type: if chunk_intact { FileValidationType::Correct } else { FileValidationType::Corrupt },
 					data_length: chunk_data_len,
 					chunk_type
 				}
@@ -112,6 +123,13 @@ impl FileValidator for PngValidator {
 
 			worst_chunk_validation = worst_chunk_validation.worst_of(chunk_info.validation_type);
 
+			if worst_chunk_validation == FileValidationType::Unrecognised {
+				break FileValidationInfo {
+					validation_type: FileValidationType::Partial,
+					file_len: Some(chunk_idx as u64 - file_match.start_idx + 12)
+				}
+			}
+
 			match chunk_info.chunk_type {
 				PNG_IHDR => {
 					seen_ihdr = true;
@@ -150,9 +168,9 @@ impl FileValidator for PngValidator {
 			} else {
 				file_data.len()
 			};
-			if chunk_idx >= max_idx {
+			if (chunk_idx + 12) >= max_idx {
 				break FileValidationInfo {
-					validation_type: FileValidationType::Corrupted,
+					validation_type: FileValidationType::Corrupt,
 					file_len: None
 				}
 			}