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

selectManyCheckbox: disabled values are accepted #4330

Closed
cnsgithub opened this Issue Feb 8, 2018 · 4 comments

Comments

Projects
None yet
2 participants
@cnsgithub

cnsgithub commented Feb 8, 2018

1) Environment

  • Version: 2.3.3

2) Expected behavior

Values of disabled components may never be accepted by the server, albeit submitted by the client.

3) Actual behavior

Select items that are disabled via f:selectItem itemDisabled="true" or SelectItem:setDisabled(true) are rendered correctly (grayed out), however may be bypassed by the client cause of missing checks on the server side. This may lead to broken access control if used for restricted areas/features.

Regarding the other SelectMany components no all-clear can be given at the moment - probably the same issue exists there.

4) Steps to reproduce

In developer tools (F12) update the disabled and checked states in the DOM. Then submit the form by pressing the button. When installing a breakpoint in Bean:setFeatures you will see that Elite was accepted as well.

5) Sample XHTML

<html xmlns="http://www.w3.org/1999/xhtml" xmlns:h="http://java.sun.com/jsf/html" xmlns:f="http://java.sun.com/jsf/core">  
	<h:head>
		<title>Testing disabled attribute</title>
	</h:head>
	<h:form>
    	Features:
	    <h:selectManyCheckbox value="#{bean.selectedFeatures}">
	    	<f:selectItems value="#{bean.availableFeatures}" />
	    </h:selectManyCheckbox>
	    
		<input type="submit" value="Submit" />
	</h:form>
</html>  

6) Sample bean

package de.test.primefaces;

import java.io.Serializable;
import javax.enterprise.context.SessionScoped;
import javax.inject.Named;

@Named("bean")
@SessionScoped
public class Bean implements Serializable {

	private List<SelectItem> availableFeatures = Arrays.asList(
			new SelectItem("Free", "Free", "Free", false), 
			new SelectItem("Elite", "Elite", "Elite", true)); //this one is disabled

	public List<String> getSelectedFeatures() {
		return selectedFeatures;
	}

	public void setSelectedFeatures(List<String> selectedFeatures) {
		this.selectedFeatures = selectedFeatures;
	}

	public List<SelectItem> getAvailableFeatures() {
		return availableFeatures;
	}

	public void setAvailableFeatures(List<SelectItem> availableFeatures) {
		this.availableFeatures = availableFeatures;
	}
	
}

7) Additional information

  • This issue was first detected in PrimeFaces (primefaces/primefaces#3264), then switching p:selectManyCheckbox to h:selectManyCheckbox revealed that mojarra also contains this security bug.
  • A similar and still existing bug was filed in mojarra (#2559). However, the issue was closed unresolved.
@BalusC

This comment has been minimized.

Collaborator

BalusC commented Feb 11, 2018

Thanks for reporting!

@BalusC BalusC closed this Feb 11, 2018

@cnsgithub

This comment has been minimized.

cnsgithub commented Feb 12, 2018

@BalusC Thanks! Is there a snapshot build that contains your fix? I just wanted to test it, however jsf-impl is not currently building properly in my environment.

@cnsgithub

This comment has been minimized.

cnsgithub commented Feb 12, 2018

@BalusC I tested your fix and can confirm that it properly works in parts.

It doesn't work for initially preselected/checked but disabled values. You may simply reproduce this by changing Issue4300.java to this:

public class Issue4330 implements Serializable {

	private String selectedValue;
	private List<String> selectedValues = new ArrayList<>(Arrays.asList("disabled"));

	public String getSelectedValue() {
		return selectedValue;
	}

	public void setSelectedValue(String selectedValue) {
		this.selectedValue = selectedValue;
	}

	public List<String> getSelectedValues() {
		return selectedValues;
	}

	public void setSelectedValues(List<String> selectedValues) {
		this.selectedValues = selectedValues;
	}

}

Then either press the submit button immediately or optionally press the hack button and check 'disabled' first, no matter - the state of the disabled value will change.

That's exactly what #2559 mentioned years ago.

Shall I open a separate issue or would you like to re-open this or the latter one?

@BalusC

This comment has been minimized.

Collaborator

BalusC commented Feb 16, 2018

It's indeed a separate issue which already exist before the fix. Let's reopen #2559.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment