JS: Avoid emitting column zero in yaml files

Author: asgerf

javascript/extractor/src/com/semmle/js/extractor/YAMLExtractor.java

@@ -3,6 +3,7 @@
 import com.semmle.util.data.StringUtil;
 import com.semmle.util.exception.CatastrophicError;
 import com.semmle.util.exception.UserError;
+import com.semmle.util.locations.LineTable;
 import com.semmle.util.trap.TrapWriter;
 import com.semmle.util.trap.TrapWriter.Label;
 import com.semmle.util.trap.TrapWriter.Table;
@@ -76,8 +77,10 @@ private static enum NodeKind {
 
   private final boolean tolerateParseErrors;
 
+  private TextualExtractor textualExtractor;
   private LocationManager locationManager;
   private TrapWriter trapWriter;
+  private LineTable lineTable;
 
   /**
    * The underlying SnakeYAML parser; we use the relatively low-level {@linkplain Parser} instead of
@@ -93,8 +96,16 @@ public YAMLExtractor(ExtractorConfig config) {
     this.tolerateParseErrors = config.isTolerateParseErrors();
   }
 
+  private LineTable getLineTable() {
+    if (lineTable == null) {
+      lineTable = new LineTable(this.textualExtractor.getSource());
+    }
+    return lineTable;
+  }
+
   @Override
   public LoCInfo extract(TextualExtractor textualExtractor) {
+    this.textualExtractor = textualExtractor;
     locationManager = textualExtractor.getLocationManager();
     trapWriter = textualExtractor.getTrapwriter();
 
@@ -253,6 +264,18 @@ private void extractLocation(Label label, Mark startMark, Mark endMark) {
     endLine = endMark.getLine() + 1;
     endColumn = endMark.getColumn();
 
+    // Avoid emitting column zero for non-empty locations
+    if (endColumn == 0 && !(startLine == endLine && startColumn == endColumn)) {
+      String source = textualExtractor.getSource();
+      int offset = getLineTable().getOffsetFromPoint(endMark.getLine(), endMark.getColumn()) - 1;
+      while (offset > 0 && isNewLine((int)source.charAt(offset))) {
+        --offset;
+      }
+      com.semmle.util.locations.Position adjustedEndPos = getLineTable().getEndPositionFromOffset(offset);
+      endLine = adjustedEndPos.getLine();
+      endColumn = adjustedEndPos.getColumn();
+    }
+
     locationManager.emitSnippetLocation(label, startLine, startColumn, endLine, endColumn);
   }
 }

javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/ExpressionInjection.expected

@@ -1,3 +1,3 @@
-| .github/workflows/comment_issue.yml:7:12:10:0 | \| | Potential injection from the github.event.comment.body context, which may be controlled by an external user. |
+| .github/workflows/comment_issue.yml:7:12:8:48 | \| | Potential injection from the github.event.comment.body context, which may be controlled by an external user. |
 | .github/workflows/comment_issue.yml:13:12:14:47 | \| | Potential injection from the github.event.comment.body context, which may be controlled by an external user. |
-| .github/workflows/comment_issue_newline.yml:9:12:11:0 | \| | Potential injection from the github.event.comment.body context, which may be controlled by an external user. |
+| .github/workflows/comment_issue_newline.yml:9:14:10:50 | \| | Potential injection from the github.event.comment.body context, which may be controlled by an external user. |