Skip to content

Commit ff94f99

Browse files
committed
*: switch to safer securejoin.Reopen
filepath-securejoin v0.3 gave us a much safer re-open primitive, we should use it to avoid any theoretical attacks. Rather than using it direcly, add a small pathrs wrapper to make libpathrs migrations in the future easier... Signed-off-by: Aleksa Sarai <[email protected]>
1 parent 6fc1914 commit ff94f99

File tree

3 files changed

+39
-8
lines changed

3 files changed

+39
-8
lines changed
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
/*
3+
* Copyright (C) 2025 Aleksa Sarai <[email protected]>
4+
* Copyright (C) 2025 SUSE LLC
5+
*
6+
* Licensed under the Apache License, Version 2.0 (the "License");
7+
* you may not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
package pathrs
20+
21+
import (
22+
"os"
23+
24+
securejoin "github.com/cyphar/filepath-securejoin"
25+
)
26+
27+
// Reopen is a wrapper around securejoin.Reopen.
28+
func Reopen(file *os.File, flags int) (*os.File, error) {
29+
return securejoin.Reopen(file, flags)
30+
}

libcontainer/exeseal/cloned_binary_linux.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/sirupsen/logrus"
1111
"golang.org/x/sys/unix"
1212

13+
"github.com/opencontainers/runc/internal/pathrs"
1314
"github.com/opencontainers/runc/libcontainer/system"
1415
)
1516

@@ -71,7 +72,7 @@ func sealFile(f **os.File) error {
7172
// When sealing an O_TMPFILE-style descriptor we need to
7273
// re-open the path as O_PATH to clear the existing write
7374
// handle we have.
74-
opath, err := os.OpenFile(fmt.Sprintf("/proc/self/fd/%d", (*f).Fd()), unix.O_PATH|unix.O_CLOEXEC, 0)
75+
opath, err := pathrs.Reopen(*f, unix.O_PATH|unix.O_CLOEXEC)
7576
if err != nil {
7677
return fmt.Errorf("reopen tmpfile: %w", err)
7778
}

libcontainer/standard_init_linux.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"golang.org/x/sys/unix"
1313

1414
"github.com/opencontainers/runc/internal/linux"
15+
"github.com/opencontainers/runc/internal/pathrs"
1516
"github.com/opencontainers/runc/libcontainer/apparmor"
1617
"github.com/opencontainers/runc/libcontainer/configs"
1718
"github.com/opencontainers/runc/libcontainer/keys"
@@ -259,19 +260,17 @@ func (l *linuxStandardInit) Init() error {
259260
return fmt.Errorf("close log pipe: %w", err)
260261
}
261262

262-
fifoPath, closer := utils.ProcThreadSelfFd(l.fifoFile.Fd())
263-
defer closer()
264-
265263
// Wait for the FIFO to be opened on the other side before exec-ing the
266264
// user process. We open it through /proc/self/fd/$fd, because the fd that
267265
// was given to us was an O_PATH fd to the fifo itself. Linux allows us to
268266
// re-open an O_PATH fd through /proc.
269-
fd, err := linux.Open(fifoPath, unix.O_WRONLY|unix.O_CLOEXEC, 0)
267+
fifoFile, err := pathrs.Reopen(l.fifoFile, unix.O_WRONLY|unix.O_CLOEXEC)
270268
if err != nil {
271-
return err
269+
return fmt.Errorf("reopen exec fifo: %w", err)
272270
}
273-
if _, err := unix.Write(fd, []byte("0")); err != nil {
274-
return &os.PathError{Op: "write exec fifo", Path: fifoPath, Err: err}
271+
defer fifoFile.Close()
272+
if _, err := fifoFile.Write([]byte("0")); err != nil {
273+
return &os.PathError{Op: "write exec fifo", Path: fifoFile.Name(), Err: err}
275274
}
276275

277276
// Close the O_PATH fifofd fd before exec because the kernel resets
@@ -280,6 +279,7 @@ func (l *linuxStandardInit) Init() error {
280279
// N.B. the core issue itself (passing dirfds to the host filesystem) has
281280
// since been resolved.
282281
// https://github.com/torvalds/linux/blob/v4.9/fs/exec.c#L1290-L1318
282+
_ = fifoFile.Close()
283283
_ = l.fifoFile.Close()
284284

285285
if s := l.config.SpecState; s != nil {

0 commit comments

Comments
 (0)