Skip to content

Inconsistent NULL handling between avifDiagnosticsClearError and avifDiagnosticsPrintf #3244

Description

@fxp

Inconsistent NULL handling between avifDiagnosticsClearError and avifDiagnosticsPrintf

In src/diag.c, two functions in the same file take a diag pointer but handle NULL inconsistently:

// src/diag.c:11-14 — no NULL check, will crash on NULL diag
void avifDiagnosticsClearError(avifDiagnostics * diag)
{
    *diag->error = '\0';
}

// src/diag.c:18-23 — explicitly handles NULL, with documenting comment
void avifDiagnosticsPrintf(avifDiagnostics * diag, const char * format, ...)
{
    if (!diag) {
        // It is possible this is NULL (e.g. calls to avifPeekCompatibleFileType())
        return;
    }
    ...
}

The comment in avifDiagnosticsPrintf documents that diag == NULL is a legitimate state — but avifDiagnosticsClearError will then segfault when called from the same code paths in that state.

avifDiagnosticsClearError is called from several public API helpers (e.g. avifImageApplyGainMap, avifCropRectFromCleanApertureBox, avifCleanApertureBoxFromCropRect, avifImageScale) that take the application's diag parameter directly — so a library user reasonably reading avifDiagnosticsPrintf and concluding "NULL is OK to pass" will hit a crash through one of these helpers.

Severity

Not a security vulnerability — the NULL state cannot be driven from attacker-controlled AVIF input, only from application misuse of the public API. Filed as a code-quality / API-consistency issue.

Suggested fix

Add the same guard at the top of avifDiagnosticsClearError:

void avifDiagnosticsClearError(avifDiagnostics * diag)
{
    if (!diag) {
        return;
    }
    *diag->error = '\0';
}

Either that, or document the NULL precondition explicitly in the public API headers and add an assert in debug builds so misuse is caught loudly.


This was surfaced by an automated LLM-driven static-extract scanner I'm experimenting with (Pipeline B of fxp/cyberai); manual review confirmed the inconsistency is real but not security-relevant. Happy to send a PR if maintainers prefer.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions