search

sudo-project / sudo

Utility to execute a command as another user
https://www.sudo.ws
Other
1.2k stars 224 forks source link

Suspicious places in the application source code #334

Closed Yugend closed 12 months ago

Yugend commented 12 months ago

I was looking through the source code of sudo and found four suspicious places: 1) The first one is in the plugins/sudoers/sudoreplay.c file, 634 line. The second statement "if" looks strange, as if it's a misprint. 2) The next place is in the same directory in tsdump.c file, 223, 228 lines. The 'first' variable is initialized True value, does not change in any way, and is then used when checking in the printf function call. 3) The third one is in the lib/protobuf-c/protobuf.c file, 2572 and 2574 lines. the pointer 'pstr' is checked twice for NULL, probably the second time it is not necessary.. 4) And the last one I found in the plugins/sample/sample_plugin.c file, 162, 172 lines. In line 172 the pointer 'cp' is used which can be NULL if the 'strchr' function does not find a colon in the 'path' variable. As far as I know, this can lead to undefined behavior.

Perhaps there is some sense in this, but I did not understand it. In that case, I'm sorry to disturb you.

sid@itb.spb.ru | "Innovative Technologies in Business" LLC | https://www.itb.spb.ru/

millert commented 12 months ago
  1. Thanks, that was an editor error. It is fixed by d17e28ad6184db8ea38b8922f860c0630753b8e9.
  2. "first" is negated after the first flag is printed, which causes subsequent flags to be separated by a comma. The static analyzer you used probably complained that "first" is always true for line 228. That is intentional as it keeps the code to print the flags consistent.
  3. That code comes from the protobuf-c project, I try to avoid making unnecessary changes.
  4. This doesn't really matter since "path" is unused if cp is NULL.