Skip to content

Commit

Permalink
fix various findings (#1200) (#1201)
Browse files Browse the repository at this point in the history
* fix issue found by @yelprofessor : avoid binding for potentially user controlled input

* csv export: escape with a tab if first char is one that would trigger the formula expansion
  • Loading branch information
syjer committed Mar 7, 2023
1 parent a00c724 commit 94e2923
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 3 deletions.
21 changes: 20 additions & 1 deletion src/main/java/alfio/util/ExportUtils.java
Expand Up @@ -20,6 +20,7 @@
import ch.digitalfondue.basicxlsx.StreamingWorkbook;
import ch.digitalfondue.basicxlsx.Style;
import com.opencsv.CSVWriter;
import org.apache.commons.lang3.StringUtils;

import javax.servlet.ServletOutputStream;
import javax.servlet.http.HttpServletResponse;
Expand Down Expand Up @@ -74,6 +75,17 @@ public static void addSheetToWorkbook(String sheetName,
}
}

// https://owasp.org/www-community/attacks/CSV_Injection
private static String escapeFormulaChar(String s) {
var trimmed = StringUtils.trimToEmpty(s);
// tab and carriage return are removed by the trimming
var res = trimmed;
if (StringUtils.startsWithAny(trimmed, "=", "+", "-", "@")) {
res = "\t" + trimmed; // http://georgemauer.net/2017/10/07/csv-injection.html starting with a tab seems to be enough?
}
return res;
}

public static void exportCsv(String fileName, String[] header, Stream<String[]> data, HttpServletResponse response) throws IOException {
response.setContentType("text/csv;charset=UTF-8");
response.setHeader("Content-Disposition", "attachment; filename=" + fileName);
Expand All @@ -83,7 +95,14 @@ public static void exportCsv(String fileName, String[] header, Stream<String[]>
out.write(marker);
}
writer.writeNext(header);
data.forEachOrdered(writer::writeNext);
data.forEachOrdered(d -> {
var copy = Arrays.copyOf(d, d.length);
for (var i = 0; i < copy.length; i++) {
var res = copy[i];
copy[i] = escapeFormulaChar(res);
}
writer.writeNext(copy);
});
writer.flush();
out.flush();
}
Expand Down
4 changes: 2 additions & 2 deletions src/main/resources/alfio/web-templates/admin-index.ms
Expand Up @@ -195,15 +195,15 @@
<ul class="nav navbar-nav visible-sm visible-xs">
<li class="nav-divider"></li>
<li class="visible-xs visible-sm">
<div class="pull-left"><a href="#" class="navbar-link" ng-click="ctrl.menuCollapsed = true" data-ui-sref="edit-current-user" title="click to update account details"><i class="fa fa-user"></i> {{username}}</a></div>
<div class="pull-left"><a href="#" class="navbar-link" ng-click="ctrl.menuCollapsed = true" data-ui-sref="edit-current-user" title="click to update account details"><i class="fa fa-user"></i> <span ng-non-bindable>{{username}}</span></a></div>
<div class="pull-right"><a href="" class="navbar-link" data-ng-click="ctrl.doLogout('{{idpLogoutRedirectionUrl}}')"><i class="fa fa-sign-out"></i> Log out</a></div>
</li>
</ul>
</div>
</div>
<div class="navbar-right hidden-sm hidden-xs">
<ul class="nav navbar-nav">
<li role="presentation" class="navbar-text"><i class="fa fa-user"></i> Logged in as {{username}}</li>
<li role="presentation" class="navbar-text"><i class="fa fa-user"></i> Logged in as <span ng-non-bindable>{{username}}</span></li>
{{#isDBAuthentication}}
<li role="presentation" data-ui-sref-active="active"> <a data-ui-sref="edit-current-user" title="click to update account details"><i class="fa fa-edit"></i> edit account</a></li>
{{/isDBAuthentication}}
Expand Down

0 comments on commit 94e2923

Please sign in to comment.