synctime: fix off-by-one read and write to a read-only buffer (Windows)

[vszakats]

Also making the --synctime option work.

Off-by-one found by Codex Security

Assisted-by: Jay Satiro

---

• Needs rework to avoid writing into a read-only buffer. Also possibly to avoid sscanf().

[vszakats]

augment review

[jay]

The data libcurl passes to the header function is not supposed to be modified

[augmentcode]

🤖 Augment PR Summary

Summary: Fixes an off-by-one bug in the Windows synctime example header callback by trimming the trailing newline at the correct index before parsing the Date: header.
Why: Prevents an out-of-bounds read/write and makes --synctime work reliably.

_{🤖 Was this summary useful? React with 👍 or 👎}

[augmentcode]

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

[Copilot]

Pull request overview

Fixes an out-of-bounds read/write in the synctime example’s header parsing on Windows, improving safety and enabling correct --synctime behavior when parsing the HTTP Date: header.

Changes:

• Corrects an off-by-one index when checking/stripping the trailing newline in the received header line.
• Updates the in-buffer NUL termination index used before sscanf() parsing.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

[vszakats]

The data libcurl passes to the header function is not supposed to be modified

Indeed, even that. It also uses sscanf which would
be nice to avoid somehow. This exceeds my C-fu.

[vszakats]

Giving a try with the local buffer.

[Copilot]

Pull request overview

Updates the Windows-only synctime libcurl example to address a header callback off-by-one access and avoid writing into the header buffer, while aiming to make --synctime functional again.

Changes:

• Fixes an off-by-one newline check when examining the received Date: header line.
• Adds logic to copy the header line into a local, NUL-terminated buffer before parsing.

Comments suppressed due to low confidence (1)

docs/examples/synctime.c:145

• The header callback buffer passed in ptr is not NUL-terminated (and should not be modified), but this code still calls sscanf(field, ...) on that non-terminated buffer. The new local header copy is correctly NUL-terminated, but it is never used for parsing; this can still lead to out-of-bounds reads. Parse from header (or otherwise ensure NUL-termination within bounds) instead of using field directly.

        char header[100];
        size_t len = nmemb < sizeof(header) ? nmemb : sizeof(header) - 1;
        memcpy(header, field, len);
        header[len] = 0; /* null-terminate local copy */
        RetVal = sscanf(field, "Date: %25s %hu %25s %hu %hu:%hu:%hu",
                        TmpStr1, &SYSTime.wDay, TmpStr2, &SYSTime.wYear,
                        &SYSTime.wHour, &SYSTime.wMinute,
                        &SYSTime.wSecond);

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

[jay]

sscanf use is fine imo. i thought it was fine in the lib as well.

[bagder]

It also uses sscanf which would be nice to avoid somehow.

It's just a tiny example, it's perfectly fine to use sscanf()!