git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: Buffer overflows

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 30, 2007, 22:14 UTC
Message-ID
<7vtzqg7jrn.fsf@gitster.siamese.dyndns.org>
In-Reply-To
<3f4fd2640708301435s7067137cp5db6334af844158a@mail.gmail.com>
"Reece Dunn" <msclrhd@googlemail.com> writes:
> Why is it easier? If you have a fixed-size buffer, why not use
> strncpy, which is what a safe string API is essentially doing anyway?

I would not claim unchecked strcpy is good -- we obviously would want to fix them.

But at the same time use of strncpy, strlcpy and friends solves only half of the problem. Often people say "use strncpy or strlcpy then you would not overstep the buffer", but that does not really solve anything, without additional logic to deal with resulting truncation (barfing with "insanely long string" error message and dying is the least impact). Continuing the work on data that the user did not intend to give you is just as wrong as using corrupt data that overflowed your static buffer.

Does Timo's nonstandard API solve that issue? Perhaps it does, perhaps not. Does it make easier to maintain our code? I highly doubt it in the current shape.

It is well and widely understood idiom to use strlcpy to a fixed-sized buffer and checking the resulting length to make sure the result would not have overflowed (and if it would have, issue an error and die). I would not have anything against a set of patches to follow such a pattern.

But a patch to add a non-standard API that nobody else uses, without any patch to show the changes to a few places that could use the API to demonstrate that the use of API vastly cleans the code up and makes it infinitely harder to make mistakes?

The API needs to justify itself to convince the people who needs to learn and adjust to that the benefit far outweighes deviation from better known patterns, and I do not see that happening in Timo's patch.

Previous: Simon 'corecode' SchubertNext: Pierre Habouzit
Message 10 of 26 in “Buffer overflows”
  1. Timo SirainenAug 30, 2007
  2. Lukas SandströmAug 30, 2007
  3. Linus TorvaldsAug 30, 2007
  4. Timo SirainenAug 30, 2007
  5. Reece DunnAug 30, 2007
  6. Timo SirainenAug 30, 2007
  7. Reece DunnAug 30, 2007
  8. Wincent ColaiutaAug 31, 2007
  9. Simon 'corecode' SchubertAug 31, 2007
  10. Junio C HamanoAug 30, 2007
  11. Pierre HabouzitAug 30, 2007
  12. Timo SirainenAug 30, 2007
  13. Johan HerlandSep 2, 2007
  14. Reece DunnSep 2, 2007
  15. David KastrupSep 2, 2007
  16. Reece DunnSep 2, 2007
  17. Jakub NarebskiSep 3, 2007
  18. Junio C HamanoSep 3, 2007
  19. René ScharfeSep 2, 2007
  20. Lukas SandströmSep 2, 2007
  21. Linus TorvaldsAug 31, 2007
  22. Timo SirainenAug 31, 2007
  23. Andreas EricssonAug 31, 2007
  24. Johannes SchindelinAug 31, 2007
  25. Temporary fix for stack smashing in mailinfoAlex Riesen, Aug 30, 2007
  26. Junio C HamanoAug 30, 2007

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.