RiffBlog

I am getting alert in Checkmarx scan saying Unsafe object binding in the saveAll() call. The exact words in checkmarx are -

The columnConfigSet at src\main\java\com\ge\digital\oa\moa\controller\ConfigController.java in line 45 may unintentionally allow setting the value of saveAll in setColumnsConfig, in the object src\main\java\com\ge\digital\oa\moa\service\ConfigService.java at line 170. 

Any idea how to rewrite the code , so that the checkmarx stops complaining.

My code:

@PutMapping("/columns") @ResponseStatus(OK) public void setColumnsConfig(@RequestBody(required=true) ColumnConfigSetDto columnConfigSet) { service.setColumnsConfig(columnConfigSet); } 

public void setColumnsConfig(ColumnConfigSetDto columnConfigSet) { String userId = columnConfigSet.getUserId(); String viewName = columnConfigSet.getViewName(); List<ColumnConfig> configs = new ArrayList<>(); for (ColumnConfigDto colConfig : columnConfigSet.getColumns()) { // build a db config row only for the visibility property for now ColumnConfigId confId = new ColumnConfigId(); confId.setUserId(userId); confId.setViewName(viewName); confId.setKey(colConfig.getKey()); confId.setProperty("visible"); ColumnConfig conf = new ColumnConfig(); conf.setColumnConfigId(confId); conf.setValue(colConfig.getIsVisible() ? "true" : "false" ); configs.add(conf); } if (!configs.isEmpty()) { configRepo.saveAll(configs); } } 

Below are my DTO Objects which is used in this code :

@Getter @Setter public class ColumnConfigSetDto { @JsonProperty("userId") private String userId; @JsonProperty("viewName") private String viewName; @JsonProperty("columns") private List<ColumnConfigDto> columns; } 

Below are my DTO code which is used in this

@Getter @Setter public class ColumnConfigDto { @JsonProperty("key") private String key; @JsonProperty("label") private String label; @JsonProperty("isVisible") private Boolean isVisible; @JsonProperty("position") private Integer position; @JsonProperty("isSortable") private Boolean isSortable; @JsonProperty("isHideable") private Boolean isHideable; } 

3 Answers

Here is my solution for Unsafe object binding reported by cherkmarx in Java. It's not a graceful approach and only fix this vulnerability.

Remove all setter methods for boxed fields in each requestbody bean. Since @JsonProperty could support deserialization capbility, no need to add setter manually.

If you need setter for request body bean indeed, you can use reflaction way instead. FieldUtils.writeField(columnConfigDto , "isVisible", true, true);

public class ColumnConfigDto { // Ensure @JsonProperty existed on each field @JsonProperty("key") private String key; @JsonProperty("isVisible") private Boolean isVisible; @JsonProperty("list") private List list; public String getKey() { return key; } public void setKey(String key) { this.key = key; } public Boolean getVisible() { return isVisible; } // Remove boxed type field // public void setVisible(Boolean visible) { // isVisible = visible; // } public List getList() { return list; } // Remove boxed type field // public void setList(List list) { // this.list = list; // } } 

this issue occurs due to @RequestBoby as per spring documentation but there is no issue for @RequestParam. if we bind request body to object without @RequestBody, this issue is not occurred.

HttpServletRequest request;

mapper.readValue(request.getInputStream(), Product.class);

The error is also thrown if data is set to an object annotated with @RequestBody.

requestBodyVariable.setAdditionalValue(valueFromRequestParamOrPathVariable); // This setter call should not be used 

Instead, use a user-defined variable for storing the value from request param, header or path variable in its place:

service.callServiceMethod(requestBodyVariable, valueFromRequestParamOrPathVariable); 

ncG1vNJzZmirpJawrLvVnqmfpJ%2Bse6S7zGiorp2jqbawutJobXFtZm2Fc4OOrqWsmZaaerCuyZ6arWWSnrultc2gZJyglZi4rq3RsQ%3D%3D