Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Quote various strings coming from untrusted sources #2408

Merged
merged 1 commit into from
May 13, 2024

Conversation

mtrmac
Copy link
Collaborator

@mtrmac mtrmac commented May 9, 2024

Typically, use %q instead of %s (or instead of "%s"), to expose various control characters and the like without interpreting them.

This is not really comprehensive; the codebase makes no general guarantee that any returned string values are free of control characters or other malicious/misleading metadata. Not even in returned "error" values (which can legitimately contain newlines, if nothing else).

A side effect of the code audit required by CVE-2024-3727 .

Typically, use %q instead of %s (or instead of "%s"), to expose
various control characters and the like without interpreting them.

This is not really comprehensive; the codebase makes no _general_
guarantee that any returned string values are free of control
characters or other malicious/misleading metadata. Not even
in returned "error" values (which can legitimately contain newlines,
if nothing else).

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
mtrmac added a commit to mtrmac/skopeo that referenced this pull request May 9, 2024
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
@mtrmac
Copy link
Collaborator Author

mtrmac commented May 9, 2024

See containers/skopeo#2325 for updated Skopeo tests.

mtrmac added a commit to mtrmac/skopeo that referenced this pull request May 13, 2024
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Copy link
Contributor

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I glanced through this and the changes I saw looked good, but I made no attempt to check or look for places that weren't changed.

@rhatdan
Copy link
Member

rhatdan commented May 13, 2024

LGTM

@rhatdan
Copy link
Member

rhatdan commented May 13, 2024

Skopeo tests need to be updated.

@mtrmac
Copy link
Collaborator Author

mtrmac commented May 13, 2024

There is a link to an updated version above.

@rhatdan rhatdan merged commit a979521 into containers:main May 13, 2024
7 of 10 checks passed
@mtrmac mtrmac deleted the quote-strings branch May 13, 2024 22:08
mtrmac added a commit to mtrmac/skopeo that referenced this pull request May 13, 2024
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
mtrmac added a commit to containers/skopeo that referenced this pull request May 13, 2024
TechIsCool pushed a commit to TechIsCool/skopeo that referenced this pull request May 15, 2024
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: David Beck <david.beck@invitae.com>
mtrmac added a commit to mtrmac/common that referenced this pull request May 22, 2024
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
TomSweeneyRedHat added a commit to TomSweeneyRedHat/common that referenced this pull request May 22, 2024
Update the libimage/pull_test.go file, changing quote in one line

Based on containers/image#2408 from @mtrmac

Signed-off-by: tomsweeneyredhat <tsweeney@redhat.com>
Luap99 added a commit to grisu48/podman that referenced this pull request May 23, 2024
The new c/image version is returning a slightly new error message[1] so
make tests use the new one.

[1] containers/image#2408

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
TomSweeneyRedHat pushed a commit to TomSweeneyRedHat/podman that referenced this pull request May 29, 2024
The new c/image version is returning a slightly new error message[1] so
make tests use the new one.

[1] containers/image#2408

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Signed-off-by: tomsweeneyredhat <tsweeney@redhat.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants