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

allow more than 1 empty AttributeDescription for ldapsearch, without the risk of denial of service #4379

Closed
tbordaz opened this issue Oct 15, 2020 · 6 comments
Assignees
Labels
easy fix Fix is easy
Milestone

Comments

@tbordaz
Copy link
Contributor

tbordaz commented Oct 15, 2020

Issue Description
#3028 enforce a strict limit of empty attributeDescription.
This is good but the limit is so low that some application may start failing
This bug is to extend a bit that limit so that application wont break

Package Version and Platform:

Steps to Reproduce

ldapsearch -LLL.... -b 'dc=example,dc=com' "" "" cn
Protocol error (2)

[15/Oct/2020:15:45:00.555817663 +0200] conn=7 op=1 SRCH base="dc=example,dc=com" scope=2 filter="(objectClass=*)", invalid attribute request

Expected results
It should succeed up to 10 empty attributes

@tbordaz tbordaz added the needs triage The issue will be triaged during scrum label Oct 15, 2020
tbordaz added a commit to tbordaz/389-ds-base that referenced this issue Oct 15, 2020
…rch, without the risk of denial of service

Bug description:
	The fix 389ds#3028 enforces a strict limit of empty attributeDescription.
        The limit is low (1) and some application may failing.
        We can relax this limit to a higher value without reopening DOS risk

Fix description:
	Change the max authorized empty attributesDescription from 1 to 10

relates: 389ds#4379

Reviewed by: Mark Reynolds

Platforms tested: F31
tbordaz added a commit that referenced this issue Oct 15, 2020
…rch, without the risk of denial of service (#4380)

Bug description:
	The fix #3028 enforces a strict limit of empty attributeDescription.
        The limit is low (1) and some application may failing.
        We can relax this limit to a higher value without reopening DOS risk

Fix description:
	Change the max authorized empty attributesDescription from 1 to 10

relates: #4379

Reviewed by: Mark Reynolds

Platforms tested: F31
@tbordaz tbordaz added this to the 1.3.10 milestone Oct 20, 2020
tbordaz added a commit that referenced this issue Oct 20, 2020
…rch, without the risk of denial of service (#4380)

Bug description:
	The fix #3028 enforces a strict limit of empty attributeDescription.
        The limit is low (1) and some application may failing.
        We can relax this limit to a higher value without reopening DOS risk

Fix description:
	Change the max authorized empty attributesDescription from 1 to 10

relates: #4379

Reviewed by: Mark Reynolds

Platforms tested: F31
tbordaz added a commit that referenced this issue Oct 20, 2020
…rch, without the risk of denial of service (#4380)

Bug description:
	The fix #3028 enforces a strict limit of empty attributeDescription.
        The limit is low (1) and some application may failing.
        We can relax this limit to a higher value without reopening DOS risk

Fix description:
	Change the max authorized empty attributesDescription from 1 to 10

relates: #4379

Reviewed by: Mark Reynolds

Platforms tested: F31
tbordaz added a commit that referenced this issue Oct 20, 2020
…rch, without the risk of denial of service (#4380)

Bug description:
	The fix #3028 enforces a strict limit of empty attributeDescription.
        The limit is low (1) and some application may failing.
        We can relax this limit to a higher value without reopening DOS risk

Fix description:
	Change the max authorized empty attributesDescription from 1 to 10

relates: #4379

Reviewed by: Mark Reynolds

Platforms tested: F31
@tbordaz
Copy link
Contributor Author

tbordaz commented Oct 20, 2020

43c6915..b8b1691 master
311dab5..4508d3c 389-ds-base-1.4.3
f397588..ab00f97 389-ds-base-1.4.2
22d6739..9faf2d7 389-ds-base-1.3.10

@tbordaz
Copy link
Contributor Author

tbordaz commented Oct 20, 2020

@tbordaz tbordaz self-assigned this Oct 20, 2020
@tbordaz tbordaz added easy fix Fix is easy and removed needs triage The issue will be triaged during scrum labels Oct 20, 2020
@tbordaz tbordaz closed this as completed Oct 20, 2020
aadhikar added a commit to aadhikar/389-ds-base that referenced this issue Jun 2, 2021
…rch, without the risk of denial of service

Desciption: Added a test case to verify up to 10 empty values and a negative
case to check max limit.

Relates: 389ds#4379

Reviewed by: @bsimonova, @droideck (Thanks!)
aadhikar added a commit to aadhikar/389-ds-base that referenced this issue Jun 2, 2021
…rch, without the risk of denial of service

Description: Added a test case to verify up to 10 empty values and a negative
case to check max limit.

Relates: 389ds#4379

Reviewed by: @vashirov, @bsimonova, @droideck (Thanks!)
bsimonova pushed a commit that referenced this issue Jun 2, 2021
…rch, without the risk of denial of service

Desciption: Added a test case to verify up to 10 empty values and a negative
case to check max limit.

Relates: #4379

Reviewed by: @bsimonova, @droideck (Thanks!)
bsimonova pushed a commit that referenced this issue Jun 2, 2021
…rch, without the risk of denial of service

Description: Added a test case to verify up to 10 empty values and a negative
case to check max limit.

Relates: #4379

Reviewed by: @vashirov, @bsimonova, @droideck (Thanks!)
@tbordaz
Copy link
Contributor Author

tbordaz commented Jun 4, 2021

@aadhikar would you mind to have a look at the recent PR that possibly creates a regression

268d1c7 Issue 4379 - Allow more than 1 empty AttributeDescription for ldapsearch, without the risk of denial of service

Test: tests/suites/acl/misc_test.py::test_info_disclosure

@tbordaz tbordaz reopened this Jun 4, 2021
@aadhikar
Copy link
Contributor

aadhikar commented Jun 4, 2021

@tbordaz by just looking at it I think it's failing because we changed the filter function to have one more argument that is attrlist which is the second argument so we need to explicitly specify scope=

@tbordaz
Copy link
Contributor Author

tbordaz commented Jun 4, 2021

@aadhikar your eyes are much better than mine. Thanks for spotting this. I will test a fix.

@tbordaz
Copy link
Contributor Author

tbordaz commented Jun 4, 2021

ff83060..f53b284 master
268d1c7..ff83060 master
53f372c..268d1c7 master

@tbordaz tbordaz closed this as completed Jun 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
easy fix Fix is easy
Projects
None yet
Development

No branches or pull requests

2 participants