Skip to content

Commit 9e24c27

Browse files
committed
Freshclam, Sigtool: Fix bug creating new files in CLD with CDIFF
The CLOSE command is failing to create a file when appending changes if the file does not already exist. This prevents adding new files to a database with a CDIFF and caused failures applying the test-3.cdiff file in the freshclam feature tests. Also improved the error message to show which command, specifically, is failing (not just the line number).
1 parent 4f1f72d commit 9e24c27

1 file changed

Lines changed: 51 additions & 11 deletions

File tree

libclamav_rust/src/cdiff.rs

Lines changed: 51 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ use std::{
2525
io::{prelude::*, BufReader, BufWriter, Read, Seek, SeekFrom, Write},
2626
iter::*,
2727
os::raw::c_char,
28-
path::PathBuf,
29-
str,
28+
path::{Path, PathBuf},
29+
str::{self, FromStr},
3030
};
3131

3232
use crate::sys;
@@ -97,8 +97,12 @@ pub enum CdiffError {
9797
///
9898
/// This error *may* wrap a processing error if the command has side effects
9999
/// (e.g., MOVE or CLOSE)
100-
#[error("{1} on line {0}")]
101-
Input(usize, InputError),
100+
#[error("{err} on line {line}: {operation}")]
101+
Input {
102+
line: usize,
103+
err: InputError,
104+
operation: String,
105+
},
102106

103107
/// An error encountered while handling a particular CDiff command
104108
#[error("processing {1} command on line {2}: {0}")]
@@ -155,14 +159,20 @@ pub enum InputError {
155159
#[error("No DB open for action {0}")]
156160
NoDBForAction(&'static str),
157161

162+
#[error("Invalid DB \"{0}\" open for action {1}")]
163+
InvalidDBForAction(String, &'static str),
164+
158165
#[error("File {0} not closed before opening {1}")]
159166
NotClosedBeforeOpening(String, String),
160167

161168
#[error("{0} not Unicode")]
162169
NotUnicode(&'static str),
163170

164-
#[error("Forbidden characters found in database name {0}")]
165-
ForbiddenCharactersInDB(String),
171+
#[error("Invalid database name {0}. Characters must be alphanumeric or '.'")]
172+
InvalidDBNameForbiddenCharacters(String),
173+
174+
#[error("Invalid database name {0}. Must not specify parent directory.")]
175+
InvalidDBNameNoParentDirectory(String),
166176

167177
#[error("{0} missing for {1}")]
168178
MissingParameter(&'static str, &'static str),
@@ -306,9 +316,20 @@ impl<'a> UnlinkOp<'a> {
306316

307317
if !db_name
308318
.chars()
309-
.all(|x: char| x.is_alphanumeric() || x == '\\' || x == '/' || x == '.')
319+
.all(|x: char| x.is_alphanumeric() || x == '.')
310320
{
311-
return Err(InputError::ForbiddenCharactersInDB(db_name.to_owned()));
321+
// DB Name contains invalid characters.
322+
return Err(InputError::InvalidDBNameForbiddenCharacters(
323+
db_name.to_owned(),
324+
));
325+
}
326+
327+
let db_path = PathBuf::from_str(db_name).unwrap();
328+
if db_path.parent() != Some(Path::new("")) {
329+
// DB Name must be not include a parent directory.
330+
return Err(InputError::InvalidDBNameNoParentDirectory(
331+
db_name.to_owned(),
332+
));
312333
}
313334

314335
Ok(UnlinkOp { db_name })
@@ -673,10 +694,22 @@ fn cmd_open(ctx: &mut Context, db_name: Option<&[u8]>) -> Result<(), InputError>
673694

674695
if !db_name
675696
.chars()
676-
.all(|x: char| x.is_alphanumeric() || x == '\\' || x == '/' || x == '.')
697+
.all(|x: char| x.is_alphanumeric() || x == '.')
677698
{
678-
return Err(InputError::ForbiddenCharactersInDB(db_name.to_owned()));
699+
// DB Name contains invalid characters.
700+
return Err(InputError::InvalidDBNameForbiddenCharacters(
701+
db_name.to_owned(),
702+
));
703+
}
704+
705+
let db_path = PathBuf::from_str(db_name).unwrap();
706+
if db_path.parent() != Some(Path::new("")) {
707+
// DB Name must be not include a parent directory.
708+
return Err(InputError::InvalidDBNameNoParentDirectory(
709+
db_name.to_owned(),
710+
));
679711
}
712+
680713
ctx.open_db = Some(db_name.to_owned());
681714

682715
Ok(())
@@ -941,6 +974,7 @@ fn cmd_close(ctx: &mut Context) -> Result<(), InputError> {
941974
// Test for lines to add
942975
if !ctx.additions.is_empty() {
943976
let mut db_file = OpenOptions::new()
977+
.create(true)
944978
.append(true)
945979
.open(&open_db)
946980
.map_err(ProcessingError::from)?;
@@ -1025,7 +1059,13 @@ where
10251059
match linebuf.first() {
10261060
// Skip comment lines
10271061
Some(b'#') => continue,
1028-
_ => process_line(ctx, &linebuf).map_err(|e| CdiffError::Input(line_no, e))?,
1062+
_ => process_line(ctx, &linebuf).map_err(|e| {
1063+
CdiffError::Input {
1064+
line: line_no,
1065+
err: e,
1066+
operation: String::from_utf8_lossy(&linebuf).to_string(),
1067+
}
1068+
})?,
10291069
}
10301070
}
10311071
}

0 commit comments

Comments
 (0)