Skip to content
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

autoGrowCollectionLimit versus general collection size limit [SPR-11472] #16097

Closed
spring-projects-issues opened this issue Feb 22, 2014 · 2 comments
Labels
in: web status: bulk-closed

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Feb 22, 2014

Dominik S. opened SPR-11472 and commented

Problem

Spring's data binder allows you to set maximum size() of automatically created List<> e.g. to 3 items. It's quite easy to bypass this limitation and cause Spring to create a List of 3000+ items simply by modifying HTTP content sent to the server.

In other words: while testing my webapp I was able by creating malicious HTTP request to force Spring's data binder to create a List<> consisting of 4000 items although I had set the limit to 3 items. This may easily lead to Out of Memory exceptions on any app server.

Version used

spring-tool-suite-3.3.0.RELEASE,
D:\m2\repo\org\springframework\spring-web\3.2.4.RELEASE\spring-web-3.2.4.RELEASE.jar

Description

I needed to bind multiple html <input /> elements to a single List<String> object, something like:

 
<input type="text" name="phoneNumber[0]" />
...
<input type="text" name="phoneNumber[n]" />

Spring performs such conversion by default using org.springframework.beans.propertyeditors.CustomCollectionEditor. Below is a simple code snippet presenting the issue described above.

Code
public class ContactDataEntity {
    private List<String> phoneNumber;
    // getters and setters
}
@RequestMapping(value = VIEW_PAGE_1, method = RequestMethod.POST)
public String xxx(HttpServletRequest request, Model model) {

	// set and bind
	ContactDataEntity contactData = new ContactDataEntity();
	ServletRequestDataBinder binder = new ServletRequestDataBinder(contactData);
	binder.setAutoGrowCollectionLimit(3); // set limit to 3 items
	binder.bind(request);

	// test binding results
	List<String> numbers = contactData.getPhoneNumber();
	if (numbers != null) {
		System.out.print("numbers SIZE: " + numbers.size() + ", DATA: ");
		for (String s : numbers) System.out.print(s + ", ");
		System.out.print("\n");
	}

	// validate and return view name...

}
Results
Results for correct data (<= 3 items, everything works ok, I use Live HTTP Headers for Firefox):

!http://i.stack.imgur.com/AEjsA.jpg!

Results for too many items (> 3 items, everything works ok, 500 Internal Server Error occurred):

!http://i.stack.imgur.com/MfWYy.jpg!

Simple trick (> 3 items, no errors reported, sorry for my typo in 'overwritten'):

!http://i.stack.imgur.com/FNlXE.jpg!

Let's exploit the above:

!http://i.stack.imgur.com/XPIhc.jpg!


Affects: 3.2.4

Reference URL: http://stackoverflow.com/questions/21962228/springs-data-binder-autogrowcollectionlimit-doesnt-work-correctly

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 5, 2014

Juergen Hoeller commented

It turns out that this is actually by design: autoGrowCollectionLimit only kicks in for auto-growing, i.e. for filling an array/collection with empty/ dummy elements up until the specified index is reached as required by an incoming parameter. This prevents growing to arbitrary collection sizes based on a single incoming parameter with a (faked) high index.

Fully populating an array/collection with explicitly specified elements in a single pass, on the other hand, is a different scenario: The incoming request contains the full set of elements in this case, and we're simply binding it to a Java data structure. While this can theoretically be turned into a memory-filling attack, it's primarily the request's large parameter structure itself then, with the data binding being a secondary problem.

So it looks like what you have in mind is a general collection size limit for data binding, even for fully populated arrays/collections. We could introduce a separate setting with such semantics; however, I wonder whether that specific case a real problem in practice. After all, large memory-consuming values can also be specified for simple Strings etc; general concerns about large incoming HTTP request bodies would have to be dealt with much earlier, before it even reaches an MVC controller.

Juergen

@spring-projects-issues spring-projects-issues added status: waiting-for-triage type: enhancement in: web and removed type: enhancement labels Jan 11, 2019
@rstoyanchev rstoyanchev added status: bulk-closed and removed status: waiting-for-triage labels Jan 11, 2019
@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jan 12, 2019

Bulk closing outdated, unresolved issues. Please, reopen if still relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web status: bulk-closed
Projects
None yet
Development

No branches or pull requests

2 participants