Skip to content

Commit

Permalink
Merge pull request #230 from CoinFabrik/scout-audit-fix
Browse files Browse the repository at this point in the history
Fixing cargo scout audit to show dylint results when used as override…
  • Loading branch information
faculerena authored Jan 17, 2024
2 parents c1eb307 + 9101d92 commit 3ec30f0
Show file tree
Hide file tree
Showing 13 changed files with 376 additions and 267 deletions.
2 changes: 1 addition & 1 deletion apps/cargo-scout-audit/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion apps/cargo-scout-audit/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "cargo-scout-audit"
version = "0.2.1"
version = "0.2.2"
edition = "2021"
authors = [
"Agustin Aon <[email protected]>",
Expand Down
12 changes: 9 additions & 3 deletions apps/cargo-scout-audit/src/startup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,10 @@ fn run_dylint(detectors_paths: Vec<PathBuf>, opts: Scout) -> Result<()> {
let stdout_temp_file = tempfile::NamedTempFile::new()?;

let is_output_stdout = opts.output_format == OutputFormat::Text && opts.output_path.is_none();
let is_output_stdout_json = opts.args.contains(&"--message-format=json".to_string());

let pipe_stdout = Some(stdout_temp_file.path().to_string_lossy().to_string());
let pipe_stderr = if is_output_stdout {
let pipe_stderr = if is_output_stdout && !is_output_stdout_json {
None
} else {
Some(stderr_temp_file.path().to_string_lossy().to_string())
Expand All @@ -190,12 +191,12 @@ fn run_dylint(detectors_paths: Vec<PathBuf>, opts: Scout) -> Result<()> {
dylint::run(&options)?;

// Format output and write to file (if necessary)
if is_output_stdout {
if is_output_stdout && !is_output_stdout_json {
return Ok(());
}

let mut stderr_file = fs::File::open(stderr_temp_file.path())?;
let stdout_file = fs::File::open(stdout_temp_file.path())?;
let mut stdout_file = fs::File::open(stdout_temp_file.path())?;

match opts.output_format {
OutputFormat::Json => {
Expand Down Expand Up @@ -223,6 +224,11 @@ fn run_dylint(detectors_paths: Vec<PathBuf>, opts: Scout) -> Result<()> {
if let Some(output_file) = opts.output_path {
let mut txt_file = fs::File::create(output_file)?;
std::io::copy(&mut stderr_file, &mut txt_file)?;
} else {
let stdout = std::io::stdout();
let mut handle = stdout.lock();
std::io::copy(&mut stdout_file, &mut handle)
.expect("Error writing dylint result to stdout");
}
}
OutputFormat::Sarif => {
Expand Down
22 changes: 12 additions & 10 deletions detectors/divide-before-multiply/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,11 +185,13 @@ fn navigate_trough_basicblocks<'tcx>(
fn_span,
..
} => {
if let Operand::Constant(cst) = func &&
let ConstantKind::Val(_, ty) = cst.literal &&
let TyKind::FnDef(id, _) = ty.kind() {
if def_ids.checked_div.is_some_and(|f|f == *id) ||
def_ids.saturating_div.is_some_and(|f|f == *id) {
if let Operand::Constant(cst) = func
&& let ConstantKind::Val(_, ty) = cst.literal
&& let TyKind::FnDef(id, _) = ty.kind()
{
if def_ids.checked_div.is_some_and(|f| f == *id)
|| def_ids.saturating_div.is_some_and(|f| f == *id)
{
tainted_places.push(*destination);
} else {
for arg in args {
Expand All @@ -198,14 +200,14 @@ fn navigate_trough_basicblocks<'tcx>(
if tainted_places.contains(place) {
tainted_places.push(*destination);

if def_ids.checked_mul.is_some_and(|f|f == *id) ||
def_ids.saturating_mul.is_some_and(|f|f == *id) {
if def_ids.checked_mul.is_some_and(|f| f == *id)
|| def_ids.saturating_mul.is_some_and(|f| f == *id)
{
spans.push(*fn_span);
}
}

},
_ => {},
}
_ => {}
}
}
}
Expand Down
36 changes: 17 additions & 19 deletions detectors/dos-unbounded-operation/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,25 +46,23 @@ struct ForLoopVisitor {
}
impl<'tcx> Visitor<'tcx> for ForLoopVisitor {
fn visit_expr(&mut self, expr: &'tcx rustc_hir::Expr<'tcx>) {
if let ExprKind::Match(match_expr, _arms, source) = expr.kind &&
source == MatchSource::ForLoopDesugar &&
let ExprKind::Call(func, args) = match_expr.kind &&
let ExprKind::Path(qpath) = &func.kind &&
let QPath::LangItem(item, _span, _id) = qpath &&
item == &LangItem::IntoIterIntoIter {

if args.first().is_some() &&
let ExprKind::Struct(qpath, fields, _) = args.first().unwrap().kind &&
let QPath::LangItem(langitem, _span, _id) = qpath &&
(
LangItem::Range == *langitem ||
LangItem::RangeInclusiveStruct == *langitem ||
LangItem::RangeInclusiveNew == *langitem
) &&
fields.last().is_some() &&
let ExprKind::Lit(lit) = &fields.last().unwrap().expr.kind &&
let LitKind::Int(_v, _typ) = lit.node {

if let ExprKind::Match(match_expr, _arms, source) = expr.kind
&& source == MatchSource::ForLoopDesugar
&& let ExprKind::Call(func, args) = match_expr.kind
&& let ExprKind::Path(qpath) = &func.kind
&& let QPath::LangItem(item, _span, _id) = qpath
&& item == &LangItem::IntoIterIntoIter
{
if args.first().is_some()
&& let ExprKind::Struct(qpath, fields, _) = args.first().unwrap().kind
&& let QPath::LangItem(langitem, _span, _id) = qpath
&& (LangItem::Range == *langitem
|| LangItem::RangeInclusiveStruct == *langitem
|| LangItem::RangeInclusiveNew == *langitem)
&& fields.last().is_some()
&& let ExprKind::Lit(lit) = &fields.last().unwrap().expr.kind
&& let LitKind::Int(_v, _typ) = lit.node
{
walk_expr(self, expr);
} else {
self.span_constant.push(expr.span);
Expand Down
90 changes: 59 additions & 31 deletions detectors/dos-unexpected-revert-with-vector/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,24 +88,47 @@ impl<'tcx> LateLintPass<'tcx> for UnexpectedRevertWarn {
if path.ident.name.to_string() == "push" {
self.push_def_id = defid;
}
} else if let ExprKind::MethodCall(rec_path, receiver2, ..) = receiver.kind &&
rec_path.ident.name.to_string() == "env" &&
let ExprKind::Path(rec2_qpath) = &receiver2.kind &&
let QPath::Resolved(qualifier, rec2_path) = rec2_qpath &&
rec2_path.segments.first().map_or(false, |seg|seg.ident.to_string() == "self" &&
qualifier.is_none()) &&
path.ident.name.to_string() == "caller" {

if self.cx.typeck_results().type_dependent_def_id(expr.hir_id).is_some() {
self.callers_def_id.insert(self.cx.typeck_results().type_dependent_def_id(expr.hir_id).unwrap());
}
} else if let ExprKind::Call(receiver2, ..) = receiver.kind &&
let ExprKind::Path(rec2_qpath) = &receiver2.kind &&
let QPath::TypeRelative(ty2, rec2_path) = rec2_qpath &&
rec2_path.ident.name.to_string() == "env" &&
let rustc_hir::TyKind::Path(rec3_qpath) = &ty2.kind &&
let QPath::Resolved(_, rec3_path) = rec3_qpath && rec3_path.segments[0].ident.to_string() == "Self" && self.cx.typeck_results().type_dependent_def_id(expr.hir_id).is_some() {
self.callers_def_id.insert(self.cx.typeck_results().type_dependent_def_id(expr.hir_id).unwrap());
} else if let ExprKind::MethodCall(rec_path, receiver2, ..) = receiver.kind
&& rec_path.ident.name.to_string() == "env"
&& let ExprKind::Path(rec2_qpath) = &receiver2.kind
&& let QPath::Resolved(qualifier, rec2_path) = rec2_qpath
&& rec2_path.segments.first().map_or(false, |seg| {
seg.ident.to_string() == "self" && qualifier.is_none()
})
&& path.ident.name.to_string() == "caller"
{
if self
.cx
.typeck_results()
.type_dependent_def_id(expr.hir_id)
.is_some()
{
self.callers_def_id.insert(
self.cx
.typeck_results()
.type_dependent_def_id(expr.hir_id)
.unwrap(),
);
}
} else if let ExprKind::Call(receiver2, ..) = receiver.kind
&& let ExprKind::Path(rec2_qpath) = &receiver2.kind
&& let QPath::TypeRelative(ty2, rec2_path) = rec2_qpath
&& rec2_path.ident.name.to_string() == "env"
&& let rustc_hir::TyKind::Path(rec3_qpath) = &ty2.kind
&& let QPath::Resolved(_, rec3_path) = rec3_qpath
&& rec3_path.segments[0].ident.to_string() == "Self"
&& self
.cx
.typeck_results()
.type_dependent_def_id(expr.hir_id)
.is_some()
{
self.callers_def_id.insert(
self.cx
.typeck_results()
.type_dependent_def_id(expr.hir_id)
.unwrap(),
);
}
}
walk_expr(self, expr);
Expand Down Expand Up @@ -142,22 +165,27 @@ impl<'tcx> LateLintPass<'tcx> for UnexpectedRevertWarn {
}
let terminator = bb_data.terminator.clone().unwrap();
if let TerminatorKind::Call { func, .. } = terminator.kind {
if let Operand::Constant(fn_const) = func &&
let ConstantKind::Val(_const_val, ty) = fn_const.literal &&
let TyKind::FnDef(def, _subs) = ty.kind() {
if !callers_def_id.is_empty() {
for caller in &callers_def_id{
if caller == def {
callers_vec.callers.push((bb_data, BasicBlock::from_usize(bb)));
}
if let Operand::Constant(fn_const) = func
&& let ConstantKind::Val(_const_val, ty) = fn_const.literal
&& let TyKind::FnDef(def, _subs) = ty.kind()
{
if !callers_def_id.is_empty() {
for caller in &callers_def_id {
if caller == def {
callers_vec
.callers
.push((bb_data, BasicBlock::from_usize(bb)));
}
} else {
for op in &push_def_id {
if op == def {
callers_vec.vec_ops.push((bb_data, BasicBlock::from_usize(bb)));
}
}
} else {
for op in &push_def_id {
if op == def {
callers_vec
.vec_ops
.push((bb_data, BasicBlock::from_usize(bb)));
}
}
}
}
}
}
Expand Down
70 changes: 34 additions & 36 deletions detectors/iterators-over-indexing/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ struct VectorAccessVisitor {

impl<'tcx> Visitor<'tcx> for VectorAccessVisitor {
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
if let ExprKind::Index(_, id) = expr.kind &&
let ExprKind::Path(qpath) = &id.kind &&
let QPath::Resolved(_, path) = qpath &&
let Res::Local(hir_id) = path.res &&
hir_id == self.index_id {

if let ExprKind::Index(_, id) = expr.kind
&& let ExprKind::Path(qpath) = &id.kind
&& let QPath::Resolved(_, path) = qpath
&& let Res::Local(hir_id) = path.res
&& hir_id == self.index_id
{
self.has_vector_access = true;
}
walk_expr(self, expr);
Expand All @@ -45,41 +45,39 @@ impl<'tcx> Visitor<'tcx> for VectorAccessVisitor {

impl<'tcx> Visitor<'tcx> for ForLoopVisitor {
fn visit_expr(&mut self, expr: &'tcx rustc_hir::Expr<'tcx>) {
if let ExprKind::Match(match_expr, arms, source) = expr.kind &&
source == MatchSource::ForLoopDesugar &&
let ExprKind::Call(func, args) = match_expr.kind &&
let ExprKind::Path(qpath) = &func.kind &&
let QPath::LangItem(item, _span, _id) = qpath &&
item == &LangItem::IntoIterIntoIter &&
args.first().is_some() &&
let ExprKind::Struct(qpath, fields, _) = args.first().unwrap().kind &&
let QPath::LangItem(langitem, _span, _id) = qpath &&
(
LangItem::Range == *langitem ||
LangItem::RangeInclusiveStruct == *langitem ||
LangItem::RangeInclusiveNew == *langitem
) &&
fields.last().is_some() &&
let ExprKind::Lit(lit) = &fields.last().unwrap().expr.kind &&
let LitKind::Int(_v, _typ) = lit.node &&
arms.first().is_some() &&
let ExprKind::Loop(block, _, loopsource, _) = arms.first().unwrap().body.kind &&
LoopSource::ForLoop == loopsource &&
block.stmts.first().is_some() &&
let StmtKind::Expr(stmtexpr) = block.stmts.first().unwrap().kind &&
let ExprKind::Match(_match_expr, some_none_arms, match_source) = stmtexpr.kind &&
MatchSource::ForLoopDesugar == match_source {

if let ExprKind::Match(match_expr, arms, source) = expr.kind
&& source == MatchSource::ForLoopDesugar
&& let ExprKind::Call(func, args) = match_expr.kind
&& let ExprKind::Path(qpath) = &func.kind
&& let QPath::LangItem(item, _span, _id) = qpath
&& item == &LangItem::IntoIterIntoIter
&& args.first().is_some()
&& let ExprKind::Struct(qpath, fields, _) = args.first().unwrap().kind
&& let QPath::LangItem(langitem, _span, _id) = qpath
&& (LangItem::Range == *langitem
|| LangItem::RangeInclusiveStruct == *langitem
|| LangItem::RangeInclusiveNew == *langitem)
&& fields.last().is_some()
&& let ExprKind::Lit(lit) = &fields.last().unwrap().expr.kind
&& let LitKind::Int(_v, _typ) = lit.node
&& arms.first().is_some()
&& let ExprKind::Loop(block, _, loopsource, _) = arms.first().unwrap().body.kind
&& LoopSource::ForLoop == loopsource
&& block.stmts.first().is_some()
&& let StmtKind::Expr(stmtexpr) = block.stmts.first().unwrap().kind
&& let ExprKind::Match(_match_expr, some_none_arms, match_source) = stmtexpr.kind
&& MatchSource::ForLoopDesugar == match_source
{
let mut visitor = VectorAccessVisitor {
has_vector_access: false,
index_id: expr.hir_id,
};
for arm in some_none_arms {
if let PatKind::Struct(qpath, pats, _) = &arm.pat.kind &&
let QPath::LangItem(item_type, _, _) = qpath &&
LangItem::OptionSome == *item_type &&
pats.last().is_some() {

if let PatKind::Struct(qpath, pats, _) = &arm.pat.kind
&& let QPath::LangItem(item_type, _, _) = qpath
&& LangItem::OptionSome == *item_type
&& pats.last().is_some()
{
if let PatKind::Binding(_, hir_id, _ident, _) = pats.last().unwrap().pat.kind {
visitor.index_id = hir_id;
walk_expr(&mut visitor, arm.body);
Expand Down
20 changes: 12 additions & 8 deletions detectors/lazy-delegate/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,17 +52,21 @@ pub struct LazyDelegate {
impl EarlyLintPass for LazyDelegate {
fn check_item(&mut self, _: &EarlyContext<'_>, item: &Item) {
if is_storage_item(item)
&& let ItemKind::Struct(strt, _) = &item.kind
&& let ItemKind::Struct(strt, _) = &item.kind
{
for field in strt.fields() {
if let Some(_) = field.ident
&& let TyKind::Path(_, path) = &field.ty.kind
&& path.segments.len() == 1
&& (path.segments[0].ident.name.to_string() == *"Lazy" || path.segments[0].ident.name.to_string() == "Mapping")
&& let Some(arg) = &path.segments[0].args
&& let GenericArgs::AngleBracketed(AngleBracketedArgs { args, .. }) = arg.clone().into_inner()
&& args.len() > 1 {} else if !self.non_lazy_manual_storage_spans.contains(&item.span){
self.non_lazy_manual_storage_spans.push(item.span);
&& let TyKind::Path(_, path) = &field.ty.kind
&& path.segments.len() == 1
&& (path.segments[0].ident.name.to_string() == *"Lazy"
|| path.segments[0].ident.name.to_string() == "Mapping")
&& let Some(arg) = &path.segments[0].args
&& let GenericArgs::AngleBracketed(AngleBracketedArgs { args, .. }) =
arg.clone().into_inner()
&& args.len() > 1
{
} else if !self.non_lazy_manual_storage_spans.contains(&item.span) {
self.non_lazy_manual_storage_spans.push(item.span);
}
}
}
Expand Down
Loading

0 comments on commit 3ec30f0

Please sign in to comment.