恶意代码漏洞 - 可能通过返回对可变对象的引用来公开内部表示

jay*_*han 19 java sonarqube

嗨,我收到的违规行为如下:

恶意代码漏洞 - 可能通过返回对可变对象的引用来公开内部表示

在我的代码中我这样写

public String[] chkBox() {
    return chkBox;
}
Run Code Online (Sandbox Code Playgroud)

我们如何解决它.

Joa*_*son 35

正如错误消息所述,您将返回内部状态(chkBox - 很可能是 - 即使您没有显示其定义,也是对象内部状态的一部分)

如果您 - 例如 - 这会导致问题

String[] box = obj.chkBox();
box[0] = null;
Run Code Online (Sandbox Code Playgroud)

由于数组对象(通过引用传递所有Java对象),因此也会更改存储在对象中的原始数组.

您最有可能想要解决的问题很简单

return (String[])chkBox.clone();
Run Code Online (Sandbox Code Playgroud)

它返回数组的副本而不是实际的数组.

  • 除了击败Findbugs之外,我不明白这是多么有用.复制的数组仍然包含对构成obj状态的字符串的引用. (2认同)
  • 这是有道理的约阿希姆,谢谢。我正在考虑这个问题。我有一个其他人编写的类,它在 getter 中返回 DomainSpecificClass[].clone()。 (2认同)

Ste*_*n C 9

我们假设如下:

  1. 从安全或隐私的角度来看,您的类会做一些重要的事情,并且状态在chkbox某种程度上用于其隐私/安全机制的类实现中.

  2. chkBox()某些不受信任的代码可以调用该方法.

现在考虑这段代码:

// ... in an untrusted method ...

Foo foo = ... 
String[] mwahaha = foo.chkBox();
mwahaha[0] = "Gotcha!"; // ... this changes the effective state of `Foo`
Run Code Online (Sandbox Code Playgroud)

通过返回对表示实际数组的引用chkbox,您可以允许Foo类外部的代码进入并更改其状态.

从设计的角度来看这很糟糕(它被称为"漏洞抽象").但是,如果在可能存在不受信任的代码的上下文中使用此类,则此(chkBox()方法)是潜在的安全漏洞. 这就是违规消息告诉你的内容.

(当然,代码检查器无法知道这个特定类是否真的对安全性至关重要.这是让你理解的.实际上对你说的是"嘿!看这里!这是可疑的!")


修复程序取决于此代码(或整个库或应用程序)是否对安全性至关重要......或者代码在将来的部署中是否具有安全性.如果这是一个误报,你可以只是抑制违规; 标记它以便检查器忽略它.如果这是一个真正的问题(或者可能成为一个真正的问题),那么要么返回一个数组的副本:

    return (String[]) chkBox.clone();
Run Code Online (Sandbox Code Playgroud)

但显然,每次调用时克隆阵列都会产生性能成本chkBox.或者,您可以修改chkBox方法以返回数组的选定元素:

    public String chkBox(int i) {
       return chkBox[i];
    }
Run Code Online (Sandbox Code Playgroud)

在这种情况下,我怀疑替代方法会更好......虽然它取决于当前如何使用该方法.