Edit read a file into a buffer it never checked the size of
Opening hello.asm showed a thirty one line file as three, one of them cut short. Opening it again hung the machine: the emulator kept running and nothing ever answered. Entry is the buffer a line is read into, and it is followed in memory by TextHead and ArenaFree - the head of the document, and the pointer its line allocator hands out. The loop that splits a file into lines copied characters in WITH NO BOUND AT ALL, so a 94 character line wrote thirteen bytes over both of them. The list head then pointed into the middle of the text and the allocator handed out an address inside the file, which is why the second open walked a list that led back into itself for ever. Typing was always safe. osReadLine is told how much room there is, so a new document behaved perfectly and a source file did not - which is exactly how the user found it, and why it looked like a mystery rather than a bug. The bound is there now, and the buffer is 128 characters: what a line is everywhere else on this machine, the same number configuration files use, rather than a second answer to a question already answered. hello.asm fits. A file with a longer line is REFUSED rather than shortened. This is an editor - a line cut on the way in would be written back cut, and the file damaged by having been looked at. It says so and exits with a status of one, which it can do since this afternoon; the file is byte identical afterwards, and the test checks that. Opened twice in the test, because once is not enough to see it: the first open does the damage and the second is what never returns. This is the third time this shape has turned up: a buffer written past its end into the variables that happened to follow it. The prompt walked off CwdText into the shell's own command names; the assembler's output ran into its label table. Every one was found by a person using the machine.
This commit is contained in:
@@ -75,8 +75,15 @@ start:
|
||||
RSTA
|
||||
STA.0
|
||||
|
||||
RSTA
|
||||
SETD.0 TooLong
|
||||
STA.0
|
||||
CALL loadFile
|
||||
|
||||
SETD.0 TooLong
|
||||
LDA.0
|
||||
BNA tooLongToEdit
|
||||
|
||||
SETD.0 FileName
|
||||
SWI osPrintString
|
||||
SETD.0 CommaText
|
||||
@@ -164,6 +171,13 @@ quit:
|
||||
RSTA
|
||||
SWI osExit
|
||||
|
||||
tooLongToEdit:
|
||||
SETD.0 TooLongText
|
||||
SWI osPrintString
|
||||
CALL newLine
|
||||
INIA 0d1
|
||||
SWI osExit
|
||||
|
||||
noName:
|
||||
SETD.0 NoNameText
|
||||
SWI osPrintString
|
||||
@@ -194,7 +208,7 @@ insertLoop:
|
||||
SETD.0 EnteringText
|
||||
SWI osPrintString
|
||||
SETD.0 Entry
|
||||
INIB 0d80
|
||||
INIB 0d128
|
||||
SWI osReadLine
|
||||
INA 0x01
|
||||
INIB 0x02 ; ENDED
|
||||
@@ -234,7 +248,7 @@ doChange:
|
||||
SETD.0 EnteringText
|
||||
SWI osPrintString
|
||||
SETD.0 Entry
|
||||
INIB 0d80
|
||||
INIB 0d128
|
||||
SWI osReadLine
|
||||
INA 0x01
|
||||
INIB 0x02 ; ENDED
|
||||
@@ -600,6 +614,29 @@ splitStep:
|
||||
XOR
|
||||
BRQ splitLine
|
||||
|
||||
; ---- Room for it ----
|
||||
;
|
||||
; THERE WAS NO CHECK HERE AT ALL, and Entry is followed in memory by TextHead and
|
||||
; ArenaFree - the head of the document and the pointer the line allocator hands out. A
|
||||
; line longer than the buffer wrote characters over both, so the list head pointed into
|
||||
; the middle of the text and the allocator handed out an address inside the file.
|
||||
;
|
||||
; What that looked like: a thirty one line file opened as three, one of them cut short.
|
||||
; Then, opening it a second time, a list that led back into itself and a machine that
|
||||
; walked it for ever - the emulator still running and the machine never answering again.
|
||||
;
|
||||
; Typing a long line was always safe, because osReadLine is told how much room there is.
|
||||
; Only the file being read went unchecked, which is why a new document behaved and a
|
||||
; source file did not.
|
||||
PSHA
|
||||
SETD.2 EntryLength
|
||||
LDA.2
|
||||
INIB 0d128
|
||||
CCF
|
||||
SUB
|
||||
POPA ; The character back, and Q still says whether there is room.
|
||||
BRQ splitTooLong
|
||||
|
||||
STA.1 ; A is still the character; an ALU operation does not touch it.
|
||||
INCD.1
|
||||
SETD.2 EntryLength
|
||||
@@ -627,6 +664,15 @@ splitOn:
|
||||
CALL takeOneOff
|
||||
BRI splitStep
|
||||
|
||||
splitTooLong:
|
||||
; NOT TRUNCATED. This is an editor: a line it shortened here would be written back
|
||||
; shortened, and the file would be damaged by having been looked at. Refusing leaves it
|
||||
; exactly as it was.
|
||||
INIA 0x01
|
||||
SETD.2 TooLong
|
||||
STA.2
|
||||
RET
|
||||
|
||||
splitLast:
|
||||
; A file that does not end in a newline still has a last line in it.
|
||||
SETD.2 EntryLength
|
||||
@@ -812,13 +858,20 @@ NeedsLineText:
|
||||
"which line?"
|
||||
NoWriteText:
|
||||
"it would not write"
|
||||
TooLongText:
|
||||
"a line in it is longer than this can edit, so it has not been opened"
|
||||
|
||||
FileName:
|
||||
#Reserve 0d24
|
||||
Command:
|
||||
#Reserve 0d41
|
||||
; A hundred and twenty eight and the zero that ends it, which is what a line is everywhere
|
||||
; else on this machine - the same number configuration files use, rather than a second
|
||||
; answer to a question already answered.
|
||||
Entry:
|
||||
#Reserve 0d81
|
||||
#Reserve 0d129
|
||||
TooLong:
|
||||
0x00
|
||||
|
||||
TextHead:
|
||||
0x00 0x00
|
||||
|
||||
Reference in New Issue
Block a user