View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0002776 | FSSCP | Build system | public | 2013-01-10 05:26 | 2013-02-16 10:43 |
| Reporter | onlyjob | Assigned To | niffiwan | ||
| Priority | low | Severity | minor | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Platform | x86_64 | OS | Debian | OS Version | 7 |
| Product Version | 3.6.14 | ||||
| Summary | 0002776: FTBFS with format-hardening [patch] | ||||
| Description | Patch to fix FTBFS: "format not a string literal" "no format arguments [-Werror=format-security]" | ||||
| Tags | No tags attached. | ||||
|
|
hardening-format.patch (11,851 bytes)
Last-Update: 2012-12-27
Author: Dmitry Smirnov <onlyjob@member.fsf.org>
Forwarded: 2013-01-10
Description: fixes FTBFS with format hardening
Workaround for error: "format not a string literal and
"no format arguments [-Werror=format-security]"
--- a/code/cutscene/cutscenes.cpp
+++ b/code/cutscene/cutscenes.cpp
@@ -319,11 +319,11 @@
if ( !rval ) {
char str[256];
if (Cmdline_nomovies)
- sprintf(str, XSTR("Movies are currently disabled.", -1));
+ sprintf(str, "%s", XSTR("Movies are currently disabled.", -1));
else
- sprintf(str, XSTR("Unable to play movie %s.", 204), Cutscenes[which_cutscene].name);
+ sprintf(str, "%s", XSTR("Unable to play movie %s.", 204), Cutscenes[which_cutscene].name);
popup(0, 1, POPUP_OK, str );
}
--- a/code/gamehelp/gameplayhelp.cpp
+++ b/code/gamehelp/gameplayhelp.cpp
@@ -119,9 +119,9 @@
ci = &Control_config[id];
if ( ci->key_id >= 0 ) {
- sprintf(buf, textify_scancode(ci->key_id));
+ sprintf(buf, "%s", textify_scancode(ci->key_id));
has_key=1;
}
if ( ci->joy_id >= 0 ) {
@@ -152,9 +152,9 @@
buf[0] = 0;
if ( ci->key_id >= 0 ) {
- sprintf(buf, textify_scancode(ci->key_id));
+ sprintf(buf, "%s", textify_scancode(ci->key_id));
has_key=1;
}
if ( ci->joy_id >= 0 ) {
--- a/code/gamesnd/eventmusic.cpp
+++ b/code/gamesnd/eventmusic.cpp
@@ -1685,12 +1685,12 @@
// NOTE: callers to this function are advised to allocate a 256 byte buffer
void event_music_get_info(char *outbuf)
{
if ( Event_music_enabled == FALSE || Event_music_level_inited == FALSE || Current_pattern == -1 ) {
- sprintf(outbuf,XSTR( "Event music is not playing", 213));
+ sprintf(outbuf, "%s", XSTR( "Event music is not playing", 213));
}
else {
- sprintf(outbuf,XSTR( "soundtrack: %s [%s]", 214), Soundtracks[Current_soundtrack_num].name, Pattern_info[Current_pattern].pattern_desc);
+ sprintf(outbuf, "%s", XSTR( "soundtrack: %s [%s]", 214), Soundtracks[Current_soundtrack_num].name, Pattern_info[Current_pattern].pattern_desc);
}
}
// ----------------------------------------------------------------
@@ -1751,9 +1751,9 @@
if ( Event_music_enabled == FALSE || Event_music_level_inited == FALSE ) {
strcpy(outbuf, XSTR( "Event music is not playing", 213));
}
else {
- sprintf(outbuf, Soundtracks[Current_soundtrack_num].name);
+ sprintf(outbuf, "%s", Soundtracks[Current_soundtrack_num].name);
}
}
// set the current soundtrack based on name
--- a/code/hud/hud.cpp
+++ b/code/hud/hud.cpp
@@ -2807,11 +2807,11 @@
}
}
if (repairing)
- sprintf(outstr, XSTR("repairing", 227));
+ sprintf(outstr, "%s", XSTR("repairing", 227));
else
- sprintf(outstr, XSTR("rearming", 228));
+ sprintf(outstr, "%s", XSTR("rearming", 228));
}
else
{
if (Player_rearm_eta > 0)
@@ -2831,32 +2831,32 @@
}
renderStringAlignCenter(position[0], position[1] + text_val_offset_y, w, outstr);
}
else if (Player_ai->ai_flags & AIF_REPAIR_OBSTRUCTED) {
- sprintf(outstr, XSTR( "obstructed", 229));
+ sprintf(outstr, "%s", XSTR( "obstructed", 229));
renderStringAlignCenter(position[0], position[1] + text_val_offset_y, w, outstr);
} else {
if ( Hud_support_objnum == -1 ) {
if (The_mission.support_ships.arrival_location == ARRIVE_FROM_DOCK_BAY)
{
- sprintf(outstr, XSTR( "exiting hangar", -1));
+ sprintf(outstr, "%s", XSTR( "exiting hangar", -1));
}
else
{
- sprintf(outstr, XSTR( "warping in", 230));
+ sprintf(outstr, "%s", XSTR( "warping in", 230));
}
renderStringAlignCenter(position[0], position[1] + text_val_offset_y, w, outstr);
} else {
ai_info *aip;
// Display "busy" when support ship isn't actually enroute to me
aip = &Ai_info[Ships[Objects[Hud_support_objnum].instance].ai_index];
if ( aip->goal_objnum != OBJ_INDEX(Player_obj) ) {
- sprintf(outstr, XSTR( "busy", 231));
+ sprintf(outstr, "%s", XSTR( "busy", 231));
show_time = 0;
} else {
- sprintf(outstr, XSTR( "dock in:", 232));
+ sprintf(outstr, "%s", XSTR( "dock in:", 232));
show_time = 1;
}
renderString(position[0] + text_dock_offset_x, position[1] + text_val_offset_y, outstr);
@@ -3815,11 +3815,11 @@
// If our ping is positive, display it
if((Netgame.server != NULL) && (Netgame.server->s_info.ping.ping_avg > 0)){
// Get the string
if(Netgame.server->s_info.ping.ping_avg >= 1000){
- sprintf(ping_str,XSTR("> 1 sec",628));
+ sprintf(ping_str, "%s", XSTR("> 1 sec",628));
} else {
- sprintf(ping_str,XSTR("%d ms",629),Netgame.server->s_info.ping.ping_avg);
+ sprintf(ping_str, "%s", XSTR("%d ms",629),Netgame.server->s_info.ping.ping_avg);
}
// Blit the string out
hud_set_default_color();
--- a/code/hud/hudtargetbox.cpp
+++ b/code/hud/hudtargetbox.cpp
@@ -388,18 +388,18 @@
// print out status of ship
switch(Current_ts) {
case TS_DIS:
- sprintf(buf,XSTR( "dis", 344));
+ sprintf(buf, "%s", XSTR( "dis", 344));
break;
case TS_OK:
- sprintf(buf,XSTR( "ok", 345));
+ sprintf(buf, "%s", XSTR( "ok", 345));
break;
case TS_DMG:
- sprintf(buf,XSTR( "dmg", 346));
+ sprintf(buf, "%s", XSTR( "dmg", 346));
break;
case TS_CRT:
- sprintf(buf,XSTR( "crt", 347));
+ sprintf(buf, "%s", XSTR( "crt", 347));
break;
}
maybeFlashElement(TBOX_FLASH_STATUS);
@@ -739,11 +739,11 @@
dist = vm_vec_dist(&target_objp->pos, &wp->homing_object->pos);
speed = vm_vec_mag(&target_objp->phys_info.vel);
if ( speed > 0 ) {
- sprintf(outstr, NOX("impact: %.1f sec"), dist/speed);
+ sprintf(outstr, "%s", NOX("impact: %.1f sec"), dist/speed);
} else {
- sprintf(outstr, XSTR( "unknown", 349));
+ sprintf(outstr, "%s", XSTR( "unknown", 349));
}
renderString(position[0] + Class_offsets[0], position[1] + Class_offsets[1], EG_TBOX_CLASS, outstr);
}
@@ -1084,9 +1084,9 @@
renderString(position[0] + order_offsets[0], position[1] + order_offsets[1], EG_TBOX_EXTRA1, outstr);
}
if ( has_orders ) {
- sprintf(outstr, XSTR( "time to: ", 338));
+ sprintf(outstr, "%s", XSTR( "time to: ", 338));
if ( ship_return_time_to_goal(tmpbuf, target_shipp) ) {
strcat_s(outstr, tmpbuf);
renderString(position[0] + time_offsets[0], position[1] + time_offsets[1], EG_TBOX_EXTRA2, outstr);
@@ -1369,11 +1369,11 @@
// print out 'disabled' on the monitor if the target is disabled
if ( (target_shipp->flags & SF_DISABLED) || (ship_subsys_disrupted(target_shipp, SUBSYSTEM_ENGINE)) ) {
if ( target_shipp->flags & SF_DISABLED ) {
- sprintf(outstr, XSTR( "DISABLED", 342));
+ sprintf(outstr, "%s", XSTR( "DISABLED", 342));
} else {
- sprintf(outstr, XSTR( "DISRUPTED", 343));
+ sprintf(outstr, "%s", XSTR( "DISRUPTED", 343));
}
gr_get_string_size(&w,&h,outstr);
renderPrintf(position[0] + Viewport_offsets[0] + Viewport_w/2 - w/2 - 1, position[1] + Viewport_offsets[1] + Viewport_h - 2*h, "%s", outstr);
}
--- a/code/network/chat_api.cpp
+++ b/code/network/chat_api.cpp
@@ -1107,9 +1107,9 @@
}
if(stricmp(szCmd,"432")==0)
{
//Channel Mode info
- snprintf(szResponse, SSIZE(szResponse), XSTR("Your nickname contains invalid characters", 640));
+ snprintf(szResponse, SSIZE(szResponse), "%s", XSTR("Your nickname contains invalid characters", 640));
AddChatCommandToQueue(CC_DISCONNECTED,NULL,0);
return szResponse;
}
if(stricmp(szCmd,"433")==0)
--- a/code/object/objectsnd.cpp
+++ b/code/object/objectsnd.cpp
@@ -121,15 +121,15 @@
sprintf(buf1,"ON");
}
if ( Objects[osp->objnum].type == OBJ_SHIP ) {
- sprintf(buf2, Ships[Objects[osp->objnum].instance].ship_name);
+ sprintf(buf2, "%s", Ships[Objects[osp->objnum].instance].ship_name);
}
else if ( Objects[osp->objnum].type == OBJ_DEBRIS ) {
- sprintf(buf2, "Debris");
+ sprintf(buf2, "%s", "Debris");
}
else {
- sprintf(buf2, "Unknown");
+ sprintf(buf2, "%s", "Unknown");
}
vec3d source_pos;
float distance;
--- a/code/playerman/playercontrol.cpp
+++ b/code/playerman/playercontrol.cpp
@@ -1445,9 +1445,9 @@
} else {
sprintf(outstr,XSTR("cargo: %s", 84), cargo_name );
}
} else {
- sprintf(outstr, XSTR( "Scanned", 85) );
+ sprintf(outstr, "%s", XSTR( "Scanned", 85) );
}
// always bash cargo_inspect_time to 0 since AI ships can reveal cargo that we
// are in the process of scanning
@@ -1463,11 +1463,11 @@
vm_vec_normalized_dir(&vec_to_cargo, &cargo_objp->pos, &Player_obj->pos);
dot = vm_vec_dot(&vec_to_cargo, &Player_obj->orient.vec.fvec);
if ( dot < CARGO_MIN_DOT_TO_REVEAL ) {
if ( !(cargo_sp->flags & SF_SCANNABLE) )
- sprintf(outstr,XSTR( "cargo: <unknown>", 86));
+ sprintf(outstr, "%s", XSTR( "cargo: <unknown>", 86));
else
- sprintf(outstr,XSTR( "not scanned", 87));
+ sprintf(outstr, "%s", XSTR( "not scanned", 87));
hud_targetbox_end_flash(TBOX_FLASH_CARGO);
Player->cargo_inspect_time = 0;
return 1;
}
@@ -1477,22 +1477,22 @@
Player->cargo_inspect_time += fl2i(frametime*1000+0.5f);
}
if ( !(cargo_sp->flags & SF_SCANNABLE) )
- sprintf(outstr,XSTR( "cargo: inspecting", 88));
+ sprintf(outstr, "%s", XSTR( "cargo: inspecting", 88));
else
- sprintf(outstr,XSTR( "scanning", 89));
+ sprintf(outstr, "%s", XSTR( "scanning", 89));
if ( Player->cargo_inspect_time > cargo_sip->scan_time ) {
ship_do_cargo_revealed( cargo_sp );
snd_play( &Snds[SND_CARGO_REVEAL], 0.0f );
Player->cargo_inspect_time = 0;
}
} else {
if ( !(cargo_sp->flags & SF_SCANNABLE) )
- sprintf(outstr,XSTR( "cargo: <unknown>", 86));
+ sprintf(outstr, "%s", XSTR( "cargo: <unknown>", 86));
else
- sprintf(outstr,XSTR( "not scanned", 87));
+ sprintf(outstr, "%s", XSTR( "not scanned", 87));
}
return 1;
}
@@ -1542,9 +1542,9 @@
} else {
sprintf(outstr,XSTR("cargo: %s", 84), cargo_name );
}
} else {
- sprintf(outstr, XSTR( "Scanned", 85) );
+ sprintf(outstr, "%s", XSTR( "Scanned", 85) );
}
// always bash cargo_inspect_time to 0 since AI ships can reveal cargo that we
// are in the process of scanning
@@ -1578,11 +1578,11 @@
subsys_in_view = hud_targetbox_subsystem_in_view(cargo_objp, &x, &y);
if ( (dot < CARGO_MIN_DOT_TO_REVEAL) || (!subsys_in_view) ) {
if ( !(cargo_sp->flags & SF_SCANNABLE) )
- sprintf(outstr,XSTR( "cargo: <unknown>", 86));
+ sprintf(outstr, "%s", XSTR( "cargo: <unknown>", 86));
else
- sprintf(outstr,XSTR( "not scanned", 87));
+ sprintf(outstr, "%s", XSTR( "not scanned", 87));
hud_targetbox_end_flash(TBOX_FLASH_CARGO);
Player->cargo_inspect_time = 0;
return 1;
}
@@ -1592,22 +1592,22 @@
Player->cargo_inspect_time += fl2i(frametime*1000+0.5f);
}
if ( !(cargo_sp->flags & SF_SCANNABLE) )
- sprintf(outstr,XSTR( "cargo: inspecting", 88));
+ sprintf(outstr, "%s", XSTR( "cargo: inspecting", 88));
else
- sprintf(outstr,XSTR( "scanning", 89));
+ sprintf(outstr, "%s", XSTR( "scanning", 89));
if ( Player->cargo_inspect_time > cargo_sip->scan_time ) {
ship_do_cap_subsys_cargo_revealed( cargo_sp, subsys, 0);
snd_play( &Snds[SND_CARGO_REVEAL], 0.0f );
Player->cargo_inspect_time = 0;
}
} else {
if ( !(cargo_sp->flags & SF_SCANNABLE) )
- sprintf(outstr,XSTR( "cargo: <unknown>", 86));
+ sprintf(outstr, "%s", XSTR( "cargo: <unknown>", 86));
else
- sprintf(outstr,XSTR( "not scanned", 87));
+ sprintf(outstr, "%s", XSTR( "not scanned", 87));
}
return 1;
}
--- a/code/ship/ship.cpp
+++ b/code/ship/ship.cpp
@@ -14007,9 +14007,9 @@
seconds = 99;
}
sprintf(outbuf, NOX("%02d:%02d"), minutes, seconds);
} else {
- sprintf( outbuf, XSTR( "Unknown", 497) );
+ sprintf( outbuf, "%s", XSTR( "Unknown", 497) );
}
return outbuf;
}
|
|
|
I am not sure if changes like this will preserve the previous functionality. - sprintf(str, XSTR("Unable to play movie %s.", 204), Cutscenes[which_cutscene].name); + sprintf(str, "%s", XSTR("Unable to play movie %s.", 204), Cutscenes[which_cutscene].name); |
|
|
Also note, I could only get one hunk to apply vs trunk. In the 1st hunk of code/hud/hudtargetbox.cpp at least, all the sprintf calls have been changed to strcpy_s (r9255). edit: actually, r9255 seems to have already fixed approx 80-90% of these. A few have been missed though. |
|
|
Thank you very much for having a look. 2Eli2: yes it should be safe as far as I can tell. 2niffiwan: Indeed in trunk there is exactly one problem with format hardening in "code/network/chat_api.cpp" -- otherwise trunk builds with format hardening as I just verified. |
|
|
Yes, I recall many of these had been fixed -- but that there was one remaining in the network chat api code. Can you provide a revised patch and we'll take a look at committing it after review? |
|
|
hardening-format[trunk].patch (769 bytes)
Last-Update: 2013-01-13
Author: Dmitry Smirnov <onlyjob@member.fsf.org>
Forwarded: 2013-01-10
Bug-Upstream: http://scp.indiegames.us/mantis/view.php?id=2776
Description: fixes FTBFS with format hardening
Workaround for error: "format not a string literal and
"no format arguments [-Werror=format-security]"
--- a/code/network/chat_api.cpp
+++ b/code/network/chat_api.cpp
@@ -1107,9 +1107,9 @@
}
if(stricmp(szCmd,"432")==0)
{
//Channel Mode info
- snprintf(szResponse, SSIZE(szResponse), XSTR("Your nickname contains invalid characters", 640));
+ snprintf(szResponse, SSIZE(szResponse), "%s", XSTR("Your nickname contains invalid characters", 640));
AddChatCommandToQueue(CC_DISCONNECTED,NULL,0);
return szResponse;
}
if(stricmp(szCmd,"433")==0)
|
|
|
Attached, thanks. I hope it's OK. |
|
|
Looks fine -- will wait for another coder to provide their view before committing to trunk. Thanks for the patch |
|
|
Looks good to me, should be fine to commit when SVN is available again. |
|
|
Fix committed to trunk@9508. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2013-01-10 05:26 | onlyjob | New Issue | |
| 2013-01-10 05:26 | onlyjob | Status | new => assigned |
| 2013-01-10 05:26 | onlyjob | Assigned To | => chief1983 |
| 2013-01-10 05:26 | onlyjob | File Added: hardening-format.patch | |
| 2013-01-10 20:27 | Eli2 | Note Added: 0014626 | |
| 2013-01-10 20:49 | niffiwan | Note Added: 0014628 | |
| 2013-01-10 20:54 | niffiwan | Note Edited: 0014628 | |
| 2013-01-11 00:08 | onlyjob | Note Added: 0014629 | |
| 2013-01-12 01:15 | Echelon9 | Note Added: 0014632 | |
| 2013-01-12 01:16 | Echelon9 | Status | assigned => code review |
| 2013-01-13 08:15 | onlyjob | File Added: hardening-format[trunk].patch | |
| 2013-01-13 08:17 | onlyjob | Note Added: 0014646 | |
| 2013-01-14 08:47 | Echelon9 | Note Added: 0014650 | |
| 2013-01-19 02:57 | niffiwan | Note Added: 0014654 | |
| 2013-01-22 09:21 | niffiwan | Changeset attached | => fs2open trunk r9508 |
| 2013-01-22 09:21 | niffiwan | Note Added: 0014658 | |
| 2013-01-22 09:21 | niffiwan | Status | code review => resolved |
| 2013-01-22 09:21 | niffiwan | Resolution | open => fixed |
| 2013-01-22 15:22 | chief1983 | Assigned To | chief1983 => niffiwan |
| 2013-01-22 15:22 | chief1983 | Status | resolved => assigned |
| 2013-01-22 15:22 | chief1983 | Status | assigned => resolved |
| 2013-02-16 10:43 | niffiwan | Relationship added | has duplicate 0002795 |
