Fatal error: Text buffer overflow

[BroadcastGames]

Inform 7 can compile and produce a .z8 file that crashes the interpreter. story here: https://raw.githubusercontent.com/BroadcastGames/Inform7_StoryCode0/master/storyFilesPile0/Inform7ZMachineStyles0.ni - run the .z8 and go North, North and it crashes. Latest version built on Ubuntu. Thank you.

North Overflow is a room, north of North Escape. "Here you go. [fixed letter spacing]A[variable letter spacing]B[bold type]C[italic type]D[roman type]E[fixed letter spacing]A[variable letter spacing]B[bold type]C[italic type]D[roman type]E[fixed letter spacing]A[variable letter spacing]B[bold type]C[italic type]D[roman type]E[fixed letter spacing]A[variable letter spacing]B[bold type]C[italic type]D[roman type]E[fixed letter spacing]A[variable letter spacing]B[bold type]C[italic type]D[roman type]E[fixed letter spacing]A[variable letter spacing]B[bold type]C[italic type]D[roman type]E[fixed letter spacing]A[variable letter spacing]B[bold type]C[italic type]D[roman type]E[fixed letter spacing]A[variable letter spacing]B[bold type]C[italic type]D[roman type]E[fixed letter spacing]A[variable letter spacing]B[bold type]C[italic type]D[roman type]E[fixed letter spacing]A[variable letter spacing]B[bold type]C[italic type]D[roman type]E[fixed letter spacing]A[variable letter spacing]B[bold type]C[italic type]D[roman type]E[fixed letter spacing]A[variable letter spacing]B[bold type]C[italic type]D[roman type]E. More: [fixed letter spacing]A[variable letter spacing]B[bold type]C[italic type]D[roman type]E. Even More: [fixed letter spacing]A[variable letter spacing]B[bold type]C[italic type]D[roman type]E[fixed letter spacing]A[variable letter spacing]B[bold type]C[italic type]D[roman type]E[fixed letter spacing]A[variable letter spacing]B[bold type]C[italic type]D[roman type]E[fixed letter spacing]A[variable letter spacing]B[bold type]C[italic type]D[roman type]E[fixed letter spacing]A[variable letter spacing]B[bold type]C[italic type]D[roman type]E[fixed letter spacing]A[variable letter spacing]B[bold type]C[italic type]D[roman type]E[fixed letter spacing]A[variable letter spacing]B[bold type]C[italic type]D[roman type]E[fixed letter spacing]A[variable letter spacing]B[bold type]C[italic type]D[roman type]E[fixed letter spacing]A[variable letter spacing]B[bold type]C[italic type]D[roman type]E[fixed letter spacing]A[variable letter spacing]B[bold type]C[italic type]D[roman type]E[fixed letter spacing]A[variable letter spacing]B[bold type]C[italic type]D[roman type]E[fixed letter spacing]A[variable letter spacing]B[bold type]C[italic type]D[roman type]E[fixed letter spacing]A[variable letter spacing]B[bold type]C[italic type]D[roman type]E[fixed letter spacing]A[variable letter spacing]B[bold type]C[italic type]D[roman type]E[fixed letter spacing]A[variable letter spacing]B[bold type]C[italic type]D[roman type]E[fixed letter spacing]A[variable letter spacing]B[bold type]C[italic type]D[roman type]E[fixed letter spacing]A[variable letter spacing]B[bold type]C[italic type]D[roman type]E[fixed letter spacing]A[variable letter spacing]B[bold type]C[italic type]D[roman type]E[fixed letter spacing]A[variable letter spacing]B[bold type]C[italic type]D[roman type]E[fixed letter spacing]A[variable letter spacing]B[bold type]C[italic type]D[roman type]E. Inform 7 build 6M62 limit. 012345678901234567890123456789."

[DavidGriffith]

I cannot get this compiled with the current release of Inform7 (6M62).

Debugging log of Inform 7
Inform called as: /usr/local/libexec/ni --internal /usr/local/share/inform7/Internal --format=ulx --project /home/dave/proj/int-fiction/frotz/test/foo.inform
Found language bundle 'Italian' (built in)
Found language bundle 'German' (built in)
Found language bundle 'Spanish' (built in)
Found language bundle 'Swedish' (built in)
Found language bundle 'English' (built in)
Found language bundle 'French' (built in)
Reading language definition from </usr/local/share/inform7/Internal/Languages/English/Syntax.preform>
720 declarations read (14183 words)

-----------------------------------------------------
Phase I ... Lexical analysis
-----------------------------------------------------

I've now read your source text, which is 935 words long.

-----------------------------------------------------
Phase II ... Semantic analysis Ia
-----------------------------------------------------

I've also read Standard Rules by Graham Nelson, which is 42655 words long.
I've also read English Language by Graham Nelson, which is 2297 words long.
I've also read Basic Screen Effects by Emily Short, which is 2218 words long.

-----------------------------------------------------
Phase III ... Initialise language semantics
-----------------------------------------------------

-----------------------------------------------------
Phase IV ... Semantic analysis Ib
-----------------------------------------------------

-----------------------------------------------------
Phase V ... Semantic analysis II
-----------------------------------------------------

-----------------------------------------------------
Phase VI ... Semantic analysis III
-----------------------------------------------------

-----------------------------------------------------
Phase VII ... First pass through assertions
-----------------------------------------------------

-----------------------------------------------------
Phase VIII ... Second pass through assertions
-----------------------------------------------------

-----------------------------------------------------
Phase IX ... Making the model world
-----------------------------------------------------

-----------------------------------------------------
Phase X ... Tables and grammar
-----------------------------------------------------

-----------------------------------------------------
Phase XI ... Phrases and rules
-----------------------------------------------------

-----------------------------------------------------
Phase XII ... Code generation
-----------------------------------------------------

==== Phase XII.1 ... Compiling the storage for the model world ====

==== Phase XII.2 ... Compiling the tables ====

==== Phase XII.3 ... Compiling the equations ====

==== Phase XII.4 ... Compiling the named action patterns ====

==== Phase XII.5 ... Compiling the action routines ====

==== Phase XII.6 ... Compiling first block of phrases ====

                    (5.a) problem message:
                    found: UNKNOWN_VNT'blue letters'
                    expected: sayable valueProblem PM_SayUnknown issued from inform7/Chapter 20/Dash.w, line 2727

                      >--> In the line 'say blue letters' (source text, line 50), I was expecting
                        that 'blue letters' would be something to 'say', but it didn't look like
                        any form of 'say' that I know. So I tried to read 'blue letters' as a value
                        of some kind (because it's legal to say values), but couldn't make sense of
                        it that way either. Sometimes this happens because punctuation has gone
                        wrong - for instance, if you've omitted a semicolon or full stop at the end
                        of the 'say' phrase.

                    (5.a) problem message:
                    found: UNKNOWN_VNT'green letters'
                    expected: sayable valueProblem PM_SayUnknown issued from inform7/Chapter 20/Dash.w, line 2727

                      >--> In the line 'say green letters' (source text, line 56), I was expecting
                        that 'green letters' would be something to 'say', but it didn't look like
                        any form of 'say' that I know. So I tried to read 'green letters' as a
                        value of some kind (because it's legal to say values), but couldn't make
                        sense of it that way either. Sometimes this happens because punctuation
                        has gone wrong - for instance, if you've omitted a semicolon or full stop
                        at the end of the 'say' phrase.

                    (5.a) problem message:
                    found: UNKNOWN_VNT'red letters'
                    expected: sayable valueProblem PM_SayUnknown issued from inform7/Chapter 20/Dash.w, line 2727

                      >--> In the line 'say red letters' (source text, line 63), I was expecting
                        that 'red letters' would be something to 'say', but it didn't look like any
                        form of 'say' that I know. So I tried to read 'red letters' as a value of
                        some kind (because it's legal to say values), but couldn't make sense of it
                        that way either. Sometimes this happens because punctuation has gone wrong
                        - for instance, if you've omitted a semicolon or full stop at the end of
                        the 'say' phrase.

                    (5.a) problem message:
                    found: UNKNOWN_VNT'red letters'
                    expected: sayable valueProblem PM_SayUnknown issued from inform7/Chapter 20/Dash.w, line 2727

                      >--> In the line 'say red letters' (source text, line 91), I was expecting
                        that 'red letters' would be something to 'say', but it didn't look like any
                        form of 'say' that I know. So I tried to read 'red letters' as a value of
                        some kind (because it's legal to say values), but couldn't make sense of it
                        that way either. Sometimes this happens because punctuation has gone wrong
                        - for instance, if you've omitted a semicolon or full stop at the end of
                        the 'say' phrase.

==== Phase XII.7 ... Compiling the rulebooks ====

==== Phase XII.8 ... Compiling scene details ====

==== Phase XII.9 ... CTNL ====

==== Phase XII.10 ... Slashing grammar (G1) ====

==== Phase XII.11 ... Determining grammar (G2) ====

-----------------------------------------------------
Phase XIII ... Compilation now complete
-----------------------------------------------------

Total of 4 files written as streams.
CPU time: 68 centiseconds

That concludes the debugging log from this run of Inform.
Its contents were as follows, and can be changed by placing
text like 'Include property creations in the debugging log.'
or 'Omit everything from the debugging log.' in the source.

Included:
  debugging log contents  debugging log inclusions
Omitted:
  action creations   action pattern compilation  action pattern parsing  assemblies    assertions    case insensitive filehandling
  conditions    constructed past participles  constructed plurals   description compilation   excerpt meanings   excerpt parsing 
  expressions    extensions census   figure creations   grammar    grammar construction   headings  
  implications    inferences    kind changes   kind checking   kind creations   lexical output 
  local variables   matching    meaning list allocation  memory allocation   noun resolution   object compilation 
  object creations   object tree   phrase comparisons   phrase compilation   phrase creations   phrase registration 
  phrase usage   predicate calculus   predicate calculus workings  pronouns    property creations   property provision 
  property translations   relation definitions   rule attachments   rulebook compilation   spatial map   spatial map workings
  specification permissions   specification usage   specificities    table construction   template reading   text substitutions 
  time periods   variable creations   verifications    vocabulary  

[BroadcastGames]

Not sure why. I'm on 6M62 also. Ubuntu 16.10 x64. Inform7ZMachineStyles0_r3_zblorb.zip

For purposes of testing Frotz, here is a compiled version attached. In the story go North, North.

[BroadcastGames]

Looking closer: Your debug log, line 2, has --format=ulx

Frotz needs .z8, right? Change your setting in Inform 7. Thank you David.

[DavidGriffith]

Confirmed with ao-curses branch.

[DavidGriffith]

Inform7ZMachineStyles0
Z-machine Glk technical demonstration by Community
Release 3 / Serial number 161225 / Inform 7 build 6M62 (I6/v6.33 lib 6/12N) SD

Place
Here is GREEN or RED text. South is Test Chamber. West is more text styles.
North has escape testing.

>n

North Escape
Here you go. One / Two // Three /// Four //// Five //// Six ////// and the other
way too. One \ Two \\ Three \\\ Four \\\\ Five \\\\\ Size \\\\\\ there. Another
room North does style changes over and over.

>n

North Overflow
Here you go. ABCDEABCDEABCDEABCDEABCDEABCDEABCDEABCDEABCDEABCDEABCDEABCDE. More:
ABCDE. Even More:
ABCDEABCDEABCDEABCDEABCDEABCDEABCDEABCDEABCDEABCDEABCDEABCDEABCDEABCDEABCDEABFaw
[Hit any key to exit.]

[DavidGriffith]

I can "fix" this by increasing TEXT_BUFFER_SIZE in src/common/frotz.h. I found that you're trying to cram 263 bytes into the output buffer. When I edited your test program to double that really lengthy string, the compiler balked, saying it was too long. What is the longest really long string legal for Inform7? I can't seem to get an Inform6 program to overflow the buffer. Xzip appears to have an output buffer size of 4000, which seems rather excessive.

So... if I change TEXT_BUFFER_SIZE to 270, will Frotz be safe from long strings like this?

[BroadcastGames]

This was the longest I could get Inform 7 6M62 to compile without restriction. I didn't test if it was the style changes or the text length itself... whatever the 263 count is from. It's not exhaustive bounds checking (I'm still relatively new to Inform 6 and Inform 7 and don't know if these bounds are known).

So... if I change TEXT_BUFFER_SIZE to 270, will Frotz be safe from long strings like this?

C/C++ code is rather notorious for allowing string buffer overflows resulting in security exploits. That was my concern for reporting it - an unchecked bounds. Not that normal users of Frotz would give it privileged OS access - but it is a somewhat higher risk situation to have a flexible virtual machine be able to escape it's I/O boundaries. TEXT_BUFFER_SIZE at 270 will solve this example (I'd punt to 512) - but I would also suggest this example points to adding a guard-check TEXT_BUFFER_SIZE before adding more to the buffer and prevent any overflow that would exit the app. Maybe just tell the player that it was truncated (message to the game output) if it goes beyond TEXT_BUFFER_SIZE?

[DavidGriffith]

I don't think this is the sort of buffer overflow that would lead to a security problem. The VM detects the overflow and aborts. There's no practical difference between that and a normal exit.

Are you suggesting converting the overflow complaint to a runtime warning and truncating the buffer to TEXT_BUFFER_SIZE length? This sort of error should not result in the VM's memory being damaged, so it might be acceptable. I'll experiment with this and commit something in a few days.

[DavidGriffith]

I'm not sure if truncating the buffer would be a good idea. We can look at the issue again later.

[DavidGriffith]

Reopening to add to "Pre-2.45 cleanup" milestone