Opened 8 years ago

Last modified 8 years ago

#876 confirmed task

Check for format-string based security holes

Reported by: ajps Owned by: ajps
Milestone: Future Keywords: cleanup
Cc:

Description

Look for more security holes like the ones fixed in [a007d2a] (SVN r1437) where we're passing an "insecure" string directly to a formatting function instead of ("%s", string).

And then fix them.

Change History (5)

comment:1 Changed 8 years ago by morth

Adding printf format attributes to z-form.h find these errors:

cmd1.c: In function ‘py_pickup_gold’:
cmd1.c:168: warning: format ‘%ld’ expects type ‘long int’, but argument 4 has type ‘s32b’

cmd5.c: In function ‘get_spell’:
cmd5.c:294: warning: unknown conversion type character ‘’ in format
cmd5.c:294: warning: unknown conversion type character ‘
’ in format
cmd5.c:369: warning: unknown conversion type character ‘’ in format

cmd5.c: In function ‘browse_spell’:
cmd5.c:456: warning: unknown conversion type character ‘’ in format
cmd5.c:463: warning: unknown conversion type character ‘
’ in format

monster/monster1.c: In function ‘output_desc_list’:
monster/monster1.c:69: warning: unknown conversion type character ‘’ in format

x-spell.c: In function ‘get_spell_info’:
x-spell.c:227: warning: spurious trailing ‘%’ in format
x-spell.c: In function ‘get_spell_info’:
x-spell.c:227: warning: spurious trailing ‘%’ in format

The caret is the uppercase conversion marked with "mega-hack", and it caused some follow up errors that I've filtered out. But cmd1.c and x-spell.c are real errors, the former most serious since it will break on 64 bit platforms.

comment:2 Changed 8 years ago by takkaria

  • Milestone changed from 3.1.1 beta to 3.1.2 beta

I've fixed those last two. We should maybe look at ways to include the format string markers all the time and avoid using the Angband-specific bits to avoid this kind of problem. Bumping to 3.1.2 now.

comment:3 Changed 8 years ago by magnate

  • Keywords cleanup added

comment:4 Changed 7 years ago by magnate

  • Milestone changed from 3.2.0 to 3.3.0

Punting to 3.3: non-urgent bug or change.

comment:5 Changed 6 years ago by magnate

  • Milestone changed from 3.3.0 to Future
  • Status changed from new to confirmed

Punting in accordance with new milestone policy (that any other milestone is only set once someone is actually working on the ticket).

Note: See TracTickets for help on using tickets.