Skip to content

Commit

Permalink
[java-source-utils] Fix lgtm java/path-injection-local (dotnet#1079)
Browse files Browse the repository at this point in the history
Context: e11d024

Commit e11d024 attempted to fix LGTM-reported
[`java/path-injection-local`][0] warnings by using the comment
`// lgtm [java/path-injection-local]`.

Unfortunately, this is insufficient: the comment *also* needs to
provide a 25+ character justification for why the offending statement
can be ignored.  This justification was not provided.

Update the `// lgtm [java/path-injection-local]` comments to provide
a justification, as required by tooling.

Copying the longer justification from e11d024:

> LGTM is complaining that `tools/java-source-utils` (69e1b80) accepts
> user-controlled data.  These warnings will be *ignored* because the
> app is *unusable* without "user-controlled data"
> …
> These are all user-controlled, and they are necessary to allow
> `java-source-utils` to *work*.
> …
> LGTM complains that `--output-javadoc FILE` accepts a user-controlled
> path which may [contain] directory separator chars, and
> *this is intentional*; using it would be annoying if that weren't true!

See also [`JavaSourceUtils.cs`][1], which passes [a value][2] located
within `$(IntermediateOutputPath)` to
`java-source-utils.jar --output-javadoc`.  Allowing `--output-javadoc`
to contain directory separator chars is what makes this possible!

[0]: https://github.com/github/codeql/blob/f192191e8c4c14d70a86342de47c8882516c7c25/java/ql/src/Security/CWE/CWE-022/TaintedPath.qhelp
[1]: https://github.com/xamarin/xamarin-android/blob/b00185c485287c2c5f0350a067ebc178aec2382c/src/Xamarin.Android.Build.Tasks/Tasks/JavaSourceUtils.cs#L134-L135
[2]: https://github.com/xamarin/xamarin-android/blob/b00185c485287c2c5f0350a067ebc178aec2382c/src/Xamarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Bindings.ClassParse.targets#L69
  • Loading branch information
jonpryor authored Jan 27, 2023
1 parent 120d8a7 commit 5fa7ac4
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ private final JavaSourceUtilsOptions parse(Iterator<String> args) throws IOExcep
final String bootClassPath = getNextOptionValue(args, arg);
final ArrayList<File> files = new ArrayList<File>();
for (final String cp : bootClassPath.split(File.pathSeparator)) {
final File file = new File(cp); // lgtm [java/path-injection-local]
final File file = new File(cp); // lgtm [java/path-injection-local] java-source-utils.jar is a command-line app, and is useless if it doesn't support command-line args.
if (!file.exists()) {
System.err.println(App.APP_NAME + ": warning: invalid file path for option `-bootclasspath`: " + cp);
continue;
Expand Down Expand Up @@ -253,7 +253,7 @@ private final JavaSourceUtilsOptions parse(Iterator<String> args) throws IOExcep
if (arg.startsWith("@")) {
// response file?
final String responseFileName = arg.substring(1);
final File responseFile = new File(responseFileName); // lgtm [java/path-injection-local]
final File responseFile = new File(responseFileName); // lgtm [java/path-injection-local] java-source-utils.jar is a command-line app, and is useless if it doesn't support command-line args.
if (responseFile.exists()) {
final Iterator<String> lines =
Files.readAllLines(responseFile.toPath())
Expand All @@ -267,7 +267,7 @@ private final JavaSourceUtilsOptions parse(Iterator<String> args) throws IOExcep
break;
}
}
final File file = new File(arg); // lgtm [java/path-injection-local]
final File file = new File(arg); // lgtm [java/path-injection-local] java-source-utils.jar is a command-line app, and is useless if it doesn't support command-line args.
if (!file.exists()) {
System.err.println(App.APP_NAME + ": warning: invalid file path for option `FILES`: " + arg);
break;
Expand Down Expand Up @@ -347,7 +347,7 @@ static File getNextOptionFile(final Iterator<String> args, final String option)
throw new IllegalArgumentException(
"Expected required value for option `" + option + "`.");
final String fileName = args.next();
final File file = new File(fileName); // lgtm [java/path-injection-local]
final File file = new File(fileName); // lgtm [java/path-injection-local] java-source-utils.jar is a command-line app, and is useless if it doesn't support command-line args.
if (!file.exists()) {
System.err.println(App.APP_NAME + ": warning: invalid file path for option `" + option + "`: " + fileName);
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public JavadocXmlGenerator(final String output) throws FileNotFoundException, Pa
if (output == null)
this.output = System.out;
else {
final File file = new File(output); // lgtm [java/path-injection-local]
final File file = new File(output); // lgtm [java/path-injection-local] java-source-utils.jar is a command-line app, and is useless if it doesn't support command-line args.
final File parent = file.getParentFile();
if (parent != null) {
parent.mkdirs();
Expand Down

0 comments on commit 5fa7ac4

Please sign in to comment.