-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Java: Big Decimal DOS #6730
base: main
Are you sure you want to change the base?
Java: Big Decimal DOS #6730
Changes from all commits
f8b3708
3e21e03
ca130db
dc83252
b6003f5
765ed91
31b5414
3e214c8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
@GetMapping("/tonghuaroot-DOSDemo01") | ||
@ResponseBody | ||
// BAD: | ||
public Long demo(@RequestParam(name = "num") BigDecimal num) { | ||
Long startTime = System.currentTimeMillis(); | ||
BigDecimal num1 = new BigDecimal(0.005); | ||
System.out.println(num1.add(num)); | ||
Long endTime = System.currentTimeMillis(); | ||
Long tempTime = (endTime - startTime); | ||
System.out.println(tempTime); | ||
return tempTime; | ||
} | ||
|
||
@GetMapping("/tonghuaroot-DOSDemo02") | ||
@ResponseBody | ||
// GOOD: | ||
public Long demo02(@RequestParam(name = "num") String num) { | ||
if (num.length() > 33 || num.matches("(?i)e")) { | ||
return "Input Parameter is too long." | ||
} | ||
Long startTime = System.currentTimeMillis(); | ||
BigDecimal num1 = new BigDecimal(0.005); | ||
System.out.println(num1.add(num)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no |
||
Long endTime = System.currentTimeMillis(); | ||
Long tempTime = (endTime - startTime); | ||
System.out.println(tempTime); | ||
return tempTime; | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,42 @@ | ||||||
<!DOCTYPE qhelp PUBLIC | ||||||
"-//Semmle//qhelp//EN" | ||||||
"qhelp.dtd"> | ||||||
<qhelp> | ||||||
|
||||||
|
||||||
<overview> | ||||||
<p>Directly incorporating user input into an BigDecimal Operation Function without validating the input | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should probably use the term "method" instead of "function" since that is the commonly used term in the Java context. |
||||||
can facilitate DOS attacks. In these attacks, the server | ||||||
will consume a lot of computing resources. A typical scenario is that the CPU usage rises to close to 100%. | ||||||
This issue often occurs in scenarios that require scientific computing, such as e-commerce platforms and electronic payments. | ||||||
</p> | ||||||
|
||||||
</overview> | ||||||
<recommendation> | ||||||
|
||||||
<p>To guard against BigDecimal DOS attacks, you should avoid putting user-provided input | ||||||
directly into a BigDecimal Method(like: add(), subtract()). Instead, In some e-commerce payment scenarios, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Maybe it would also be worth formatting these methods as code using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @Marcono1234 help review, I will to do some modify about this query rule's help file. |
||||||
we should verify the size of the incoming number. For example, under normal circumstances, | ||||||
the number representing money will not contain "e".</p> | ||||||
|
||||||
</recommendation> | ||||||
<example> | ||||||
|
||||||
<p>The following example shows a BigDecimal parameter being directly passed | ||||||
to a BigDecimal method (like: add(), subtract()) without validating the input, which facilitates DOS attacks. | ||||||
It also shows how to remedy the problem by validating the user input against a known fixed string. | ||||||
</p> | ||||||
|
||||||
<sample src="BigDecimalDOS.java" /> | ||||||
|
||||||
</example> | ||||||
<references> | ||||||
<li> | ||||||
<a href="https://www.cuijianxiong.top/2021/03/30/%E6%9A%B4%E5%8A%9B%E7%9A%84%E6%94%BB%E5%87%BBDos/">暴力的攻击-Dos</a> | ||||||
</li> | ||||||
<li> | ||||||
<a href="https://mp.weixin.qq.com/s/4YX7XA34fShdCpGawO-Q_Q">BigDecimal DOS</a> | ||||||
Comment on lines
+35
to
+38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might be good to add "(Chinese)" or a similar indicator to the link text; otherwise it might be confusing that the pages are in Chinese (?) because the query help text is in English. Not sure if there are any good blog posts written in English, but maybe the following is relevant? |
||||||
</li> | ||||||
|
||||||
</references> | ||||||
</qhelp> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
/** | ||
* @id java/BigDecimalDOS | ||
* @name Java-BigDecimal-DOS-Vulnerability | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The name does not have to use hyphens, see query metadata style guide |
||
* @description When user-controllable data is passed to the relevant methods in BigDecimal, it may cause DOS issues when computing resources are limited. This issue is common in business scenarios such as e-commerce platforms. | ||
* @kind path-problem | ||
* @problem.severity error | ||
*/ | ||
|
||
import java | ||
import semmle.code.java.dataflow.FlowSources | ||
import DataFlow::PathGraph | ||
|
||
class BigDecimalDOSConfig extends TaintTracking::Configuration { | ||
BigDecimalDOSConfig() { this = "Java-BigDecimal-DOS-Vulnerability-Config" } | ||
|
||
override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource } | ||
|
||
override predicate isSink(DataFlow::Node sink) { | ||
exists(Method method, MethodAccess call | | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @atorralba for helping review this part of the code. |
||
method.getDeclaringType().hasQualifiedName("java.math", "BigDecimal") and | ||
method.hasName(["add", "subtract"]) and | ||
call.getMethod() = method and | ||
sink.asExpr() = call.getArgument(0) | ||
) | ||
} | ||
} | ||
|
||
from BigDecimalDOSConfig config, DataFlow::PathNode source, DataFlow::PathNode sink | ||
where config.hasFlowPath(source, sink) | ||
select source.getNode(), source, sink, "Potential Java BigDecimal DOS issue due to $@.", | ||
source.getNode(), "a user-provided value" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
edges | ||
| BigDecimalDOS.java:12:28:12:69 | num : BigDecimal | BigDecimalDOS.java:14:30:14:32 | num | | ||
nodes | ||
| BigDecimalDOS.java:12:28:12:69 | num : BigDecimal | semmle.label | num : BigDecimal | | ||
| BigDecimalDOS.java:14:30:14:32 | num | semmle.label | num | | ||
subpaths | ||
#select | ||
| BigDecimalDOS.java:12:28:12:69 | num | BigDecimalDOS.java:12:28:12:69 | num : BigDecimal | BigDecimalDOS.java:14:30:14:32 | num | Potential Java BigDecimal DOS Issue due to $@. | BigDecimalDOS.java:12:28:12:69 | num | a user-provided value | |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
package com.example.servingwebcontent; | ||
|
||
import org.springframework.stereotype.Controller; | ||
import org.springframework.ui.Model; | ||
import org.springframework.web.bind.annotation.GetMapping; | ||
import org.springframework.web.bind.annotation.RequestParam; | ||
import org.springframework.web.bind.annotation.ResponseBody; | ||
|
||
import java.math.BigDecimal; | ||
import java.math.MathContext; | ||
|
||
@Controller | ||
public class GreetingController { | ||
@GetMapping("/tonghuaroot-DOSDemo01") | ||
@ResponseBody | ||
// BAD: | ||
public Long demo(@RequestParam(name = "num") BigDecimal num) { | ||
Long startTime = System.currentTimeMillis(); | ||
BigDecimal num1 = new BigDecimal(0.005); | ||
System.out.println(num1.add(num)); | ||
Long endTime = System.currentTimeMillis(); | ||
Long tempTime = (endTime - startTime); | ||
System.out.println(tempTime); | ||
return tempTime; | ||
} | ||
|
||
@GetMapping("/tonghuaroot-DOSDemo02") | ||
@ResponseBody | ||
// GOOD: | ||
public Long demo02(@RequestParam(name = "num") String num) { | ||
if (num.length() > 33 || num.matches("(?i)e")) { | ||
return "Input Parameter is too long." | ||
} | ||
Long startTime = System.currentTimeMillis(); | ||
BigDecimal num1 = new BigDecimal(0.005); | ||
System.out.println(num1.add(num)); | ||
Long endTime = System.currentTimeMillis(); | ||
Long tempTime = (endTime - startTime); | ||
System.out.println(tempTime); | ||
return tempTime; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
experimental/Security/BigDecimalDOS/BigDecimalDOS.ql |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/servlet-api-2.4:${testdir}/../../../../stubs/springframework-5.3.8/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't that be:
(or similar)