Dear Maintainer,
"steghide embed" intermittently reads freed memory while computing the
matching, and on some platforms segfaults. The embed RNG is seeded from
/dev/urandom, so the same command crashes on some runs and succeeds on others.
The bug is present in 0.5.1+git20240220-2 (sid/forky) and in 0.5.1-15
(bookworm/trixie), and current upstream (github.com/StegHigh/steghide) has the
same code.
Reproduction (debian:sid container, arm64, steghide 0.5.1+git20240220-2 with
steghide-dbgsym):
python3 -c 'import random, struct; open("cover.au", "wb").write(struct.pack(">6I", 0x2e736e64, 24, 32000, 1, 8000, 1) + random.Random(546).randbytes(32000))'
printf 'north of the old bridge\n' > msg.txt
for i in $(seq 1 40); do
valgrind -q steghide embed -cf cover.au -sf out.au -ef msg.txt -p "" -f -q 2>vg.$i >/dev/null
done
grep -l "Invalid read" vg.* | wc -l
10 of 40 runs report the invalid read below. With glibc the stale read usually
goes unnoticed, so the native binary did not crash in 100 runs. A native macOS
arm64 build of the 0.5.1-15 sources segfaults on 17 of 100 plain runs of the
same command, and an AddressSanitizer build reports heap-use-after-free with
the same stacks. 16-bit PCM WAV and AU covers crash too, at a lower rate (about
4 of 100 with a 256-byte payload).
Invalid read of size 8
at 0x401DA74: operator++ (stl_list.h:708)
by 0x401DA74: EdgeIterator::operator++() (EdgeIterator.cc:72)
by 0x401040F: DFSAPHeuristic::getNextEdge(Vertex*) (DFSAPHeuristic.cc:227)
by 0x4010637: DFSAPHeuristic::searchAugmentingPath(Vertex*, Edge const**) (DFSAPHeuristic.cc:155)
by 0x401086F: DFSAPHeuristic::run() (DFSAPHeuristic.cc:81)
by 0x401F5CF: Embedder::calculateMatching(ProgressOutput*) (Embedder.cc:221)
by 0x4020757: Embedder::embed() (Embedder.cc:168)
by 0x4031BEF: Session::run() (Session.cc:40)
by 0x40069FB: main (main.cc:95)
Address 0x5162aa0 is 0 bytes inside a block of size 32 free'd
at 0x48E9774: operator delete(void*, unsigned long) (vg_replace_malloc.c:1146)
by 0x40233C3: erase (list.tcc:153)
by 0x40233C3: Graph::markDeletedSampleOccurence(std::_List_iterator<SampleOccurence>) (Graph.cc:199)
by 0x40333DF: Vertex::markDeleted() (Vertex.cc:97)
by 0x403A597: SMDConstructionHeuristic::run() (SMDConstructionHeuristic.cc:51)
by 0x401F5CF: Embedder::calculateMatching(ProgressOutput*) (Embedder.cc:221)
by 0x4020757: Embedder::embed() (Embedder.cc:168)
by 0x4031BEF: Session::run() (Session.cc:40)
by 0x40069FB: main (main.cc:95)
Cause: CvrStgFile::getMatchingAlgorithms() (and the AuFile override)
constructs every algorithm before any of them runs. The DFSAPHeuristic
constructor resets one EdgeIterator per vertex, which stores iterators into
Graph::SampleOccurences. The construction heuristic runs next (SMD for AU,
WKS otherwise): Vertex::markDeleted() erases those list nodes and
Graph::unmarkDeletedAllVertices() inserts new ones. searchAugmentingPath()
marks a matched vertex visited without resetting its iterator, so a later
getNextEdge() on that vertex takes the "already visited" branch and advances
the stale iterator.
The attached patch resets every edge iterator at the start of
DFSAPHeuristic::run(), after the construction heuristic has finished. I built
0.5.1+git20240220-2 with it added to debian/patches/series. Valgrind then
reported no invalid reads in 40 runs of the reproduction above (10 of 40
before), and every output extracted back to the original message.
Independently, an AddressSanitizer build of the 0.5.1-15 sources with the
patch produced 400 clean embed/extract round trips on AU and WAV covers
(unpatched: 29 use-after-free reports in 200 runs).