Skip to content

kfuzztest: string null-termination #6365

@ethangraham2001

Description

@ethangraham2001

Strings should always be null terminated for KFuzzTest calls,
as these can trigger false positive overflows (e.g., when calling
strlen).

The ideal place to do this is probably directly where the arguments
are generated and mutated. This was implemented like so in randString:

syzkaller/prog/rand.go

Lines 362 to 366 in cd97285

// We always null-terminate strings that are inputs to KFuzzTest calls to
// avoid false-positive buffer overflow reports.
if r.oneOf(100) == t.NoZ || r.genKFuzzTest {
buf.Write([]byte{0})
}

However, it has been observed that there are cases where syzkaller will
still generate non-null-terminated strings. As a quick fix for this that
guarantees null-termination, we forcibly null-terminate strings during
KFuzzTest argument encoding like so:

syzkaller/prog/kfuzztest.go

Lines 169 to 176 in 81f2154

// Unlike length fields whose incorrectness can be prevented easily,
// it is an invasive change to prevent generation of
// non-null-terminated strings. Forcibly null-terminating them
// during encoding allows us to centralize this easily and prevent
// false positive buffer overflows in KFuzzTest targets.
if buffer.Kind == BufferString && (len(data) == 0 || data[len(data)-1] != byte(0)) {
data = append(data, byte(0))
}

Despite working just fine, it would be better to do this in prog/rand.go.

References #6280

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions