Skip to content

Commit

Permalink
validate-vuln: bugfixing
Browse files Browse the repository at this point in the history
1. Compute deathSignal correctly
2. Redirect output to tmp file.
   Very long piped strings sometimes get mishandled.
  • Loading branch information
davisjam committed Feb 3, 2019
1 parent e34b2bc commit f15e61e
Showing 1 changed file with 22 additions and 7 deletions.
29 changes: 22 additions & 7 deletions src/validate/validate-vuln.pl
Original file line number Diff line number Diff line change
Expand Up @@ -80,19 +80,19 @@
my $input = { "pattern" => $json->{pattern},
"input" => $attackString,
};
my $tmpFile = "/tmp/validate-vuln-$$.json";
&writeToFile("file"=>$tmpFile, "contents"=>encode_json($input));
my $tmpQueryFile = "/tmp/validate-vuln-$$-queryFile.json";
my $tmpStdoutFile = "/tmp/validate-vuln-$$-validator-stdout.json";
&writeToFile("file"=>$tmpQueryFile, "contents"=>encode_json($input));

# Invoke the appropriate validator.
my $validator = $language2validator{$json->{language}};

# Use KILL because Ruby blocks TERM during regex match (??).
my ($rc, $deathSignal, $out) = &cmd("timeout --signal=KILL $json->{timeLimit}s $validator $tmpFile");
unlink $tmpFile;
my ($rc, $deathSignal, $out) = &cmd("timeout --signal=KILL $json->{timeLimit}s $validator $tmpQueryFile > $tmpStdoutFile");
# On timeout, rc is 124 if using TERM and 128+9 if using KILL
# The right-shift of 8 in &cmd turns 128+9 into 9
my $timedOut = ($rc eq 124 or $deathSignal eq 9) ? 1 : 0;
&log("rc $rc timedOut $timedOut out\n $out");
&log("rc $rc deathSignal $deathSignal timedOut $timedOut");

# Append appropriate values to $result
if ($timedOut) {
Expand All @@ -105,16 +105,31 @@

# If it didn't time out, we should have valid JSON output.
# Was the regex valid?
my $validatorRes = decode_json($out);
my $content = &slurpFile($tmpStdoutFile);
my $validatorRes = decode_json($content);
$result->{validPattern} = $validatorRes->{validPattern};
}

unlink $tmpQueryFile;
unlink $tmpStdoutFile;
}

print STDOUT encode_json($result) . "\n";
exit 0;

######################

sub slurpFile {
my ($file) = @_;
{
open F, $file or die "Can't read $file: $!";
local $/; # enable slurp mode, locally.
my $contents = <F>;
close F;
return $contents;
}
}

# input: %args: keys: file contents
# output: $file
sub writeToFile {
Expand All @@ -133,8 +148,8 @@ sub cmd {
&log($cmd);
my $out = `$cmd`;

my $deathSignal = $? & 127;
my $rc = $? >> 8;
my $deathSignal = $rc & 127;

return ($rc, $deathSignal, $out);
}
Expand Down

2 comments on commit f15e61e

@jamesdonoh
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re very long piped strings sometimes get mishandled: I hit the same problem, for the record I eventually tracked it down to nodejs/node#6456 and nodejs/node#1741 (comment)

My fix was to simply delete the call to process.exit at https://github.com/davisjam/vuln-regex-detector/blob/master/src/validate/src/javascript/query-node.js#L60 but then I noticed you've already fixed it upstream :-)

@davisjam
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, really interesting that it was due to something in Node. Unexpected!

Please sign in to comment.