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

Better sanitation for variables fetch from pam_get_item() #99

Closed
baszoetekouw opened this issue Oct 11, 2024 · 2 comments · Fixed by #103
Closed

Better sanitation for variables fetch from pam_get_item() #99

baszoetekouw opened this issue Oct 11, 2024 · 2 comments · Fixed by #103
Assignees
Labels
bug Something isn't working

Comments

@baszoetekouw
Copy link
Member

It's unclear from the pam_get_item() documentation on what type of sanitation it does, so we can't be sure that it outputs valid strings.

So, add a wrapper function that fetches a var from pam_get_item(), copies it to a fixed-length buffer (1024 bytes should be enough for everyone!), and checks that there a no weird characters in there. At least remove ", {, and }, because they are sensitive for the output json.

@baszoetekouw baszoetekouw added the bug Something isn't working label Oct 11, 2024
@baszoetekouw baszoetekouw moved this from New to Needs refinement in SRAM development Oct 11, 2024
@baszoetekouw baszoetekouw moved this from Needs refinement to Todo in SRAM development Oct 11, 2024
@mrvanes mrvanes linked a pull request Oct 18, 2024 that will close this issue
@marlam
Copy link
Contributor

marlam commented Oct 18, 2024

This is related to PR #103 ; sorry that I did not see this issue earlier.

PR #103 checks the input in a separate function input_is_safe(). This could also be done in a wrapper around tty_input() or in tty_input() itself, but it does not seem to be strictly necessary for group input since that is parsed by strtol() which already catches errors, and input_is_safe() can also be used on input from other sources, e.g. the username.

I see no remaining cases of user or system input that is not validated. There is also input from the web service, but that needs to be trustworthy anyway.

@sram-project-automation sram-project-automation bot moved this from In progress to To be tested in SRAM development Nov 13, 2024
@mrvanes
Copy link
Contributor

mrvanes commented Nov 13, 2024

'aa}bb' rejected
'aa{bb' rejected
"aa"bb" rejected
"a(*255)" accepted (MAX_INPUT_LENGTH = 256)
"a(*256)" rejected

OK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: To be deployed
Development

Successfully merging a pull request may close this issue.

3 participants